Merge lp:~acsone-openerp/openerp-connector/7.0-connector-parameterized-job-desciption into lp:~openerp-connector-core-editors/openerp-connector/7.0

Proposed by Laurent Mignon (Acsone)
Status: Merged
Merged at revision: 598
Proposed branch: lp:~acsone-openerp/openerp-connector/7.0-connector-parameterized-job-desciption
Merge into: lp:~openerp-connector-core-editors/openerp-connector/7.0
Diff against target: 155 lines (+39/-8)
3 files modified
connector/CHANGES.rst (+3/-0)
connector/queue/job.py (+16/-7)
connector/tests/test_job.py (+20/-1)
To merge this branch: bzr merge lp:~acsone-openerp/openerp-connector/7.0-connector-parameterized-job-desciption
Reviewer Review Type Date Requested Status
Stéphane Bidoul (Acsone) (community) Approve
Guewen Baconnier @ Camptocamp code review Approve
Review via email: mp+195572@code.launchpad.net

Description of the change

Improve Job description
=======================

Jobs are tasks to execute later. While the functionality is really robust and performs well, it is not always easy for the end user to visually distinguish the purpose of a job in the list of jobs. The description displayed in the list is computed from the documentation of the delayed function or from its name and there is no way to particularize this message by job instance.

This proposal aims to address this limitation by adding to the delay function a new optional parameter 'description'. The description given in the call to the delay function is used and displayed for the description of the job instead of the default one.

This proposal is backward compatible.

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

This is a very interesting improvement and well done!
Thanks

review: Approve (code review)
Revision history for this message
Laurent Mignon (Acsone) (lmi) wrote :

It's nice to contribute ;-)

On Mon, Nov 18, 2013 at 11:29 AM, Guewen Baconnier @ Camptocamp <
<email address hidden>> wrote:

> Review: Approve code review
>
> This is a very interesting improvement and well done!
> Thanks
> --
>
> https://code.launchpad.net/~acsone-openerp/openerp-connector/7.0-connector-parameterized-job-desciption/+merge/195572
> Your team Acsone OpenErp Team is subscribed to branch
> lp:~acsone-openerp/openerp-connector/7.0-connector-parameterized-job-desciption.
>

--
*Laurent Mignon*
*Senior Software Engineer*

*Tel : +352 20 21 10 20 32*
*Fax : +352 20 21 10 21*
*Gsm : +352 691 506 009*
*Email: <email address hidden> <email address hidden>*

*Acsone SA, Succursale de Luxembourg*
*22, Zone industrielle*
*L-8287 Kehlen, Luxembourg*
*www.acsone.eu <http://www.acsone.eu>*

Revision history for this message
David BEAL (ak) (davidbeal) wrote :

Good idea,

But perhaps we may use 'description' argument as complement of self.func.__doc__ or 'Function %s' and not instead of

Revision history for this message
Stéphane Bidoul (Acsone) (sbi) wrote :

This is working quite nicely in production already.

Nitpicking:
- * description : a human description of the job
+ * description : a human-readable description of the job,
+ intended to discriminate job instances

review: Approve
600. By Laurent Mignon (Acsone)

little doc improvement as suggested by Stéphane Bidoul (Acsone) (sbi)

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

David Beal:
> Good idea,
>
> But perhaps we may use 'description' argument as complement of
> self.func.__doc__ or 'Function %s' and not instead of

Perhaps can you precise what you have in mind with some examples?
Appending them would give somewhat weird results (redundancy, unrelated messages).

We must keep things simple, this proposal stays simple while still allowing to fully customize the job descriptions. That's why a totally support it.

Laurent Mignon
> It's nice to contribute ;-)

Even more when the contribution is scrupulous about details (including tests, docs, ...) :-)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'connector/CHANGES.rst'
2--- connector/CHANGES.rst 2013-10-09 18:52:36 +0000
3+++ connector/CHANGES.rst 2013-11-18 14:29:22 +0000
4@@ -4,6 +4,9 @@
5 2.0.1.dev0
6 ~~~~~~~~~~
7
8+* Add a new optional keyword argument 'description' to the delay function of a job. If given, the description is used as name of the queue.job model stored in OpenErp
9+and displayed in the list of jobs.
10+
11 2.0.1 (2013-09-12)
12 ~~~~~~~~~~~~~~~~~~
13
14
15=== modified file 'connector/queue/job.py'
16--- connector/queue/job.py 2013-10-28 15:17:17 +0000
17+++ connector/queue/job.py 2013-11-18 14:29:22 +0000
18@@ -102,9 +102,9 @@
19 "Model %s not found" % self._job_model_name)
20
21 def enqueue(self, func, model_name=None, args=None, kwargs=None,
22- priority=None, eta=None, max_retries=None):
23+ priority=None, eta=None, max_retries=None, description=None):
24 job = Job(func=func, model_name=model_name, args=args, kwargs=kwargs,
25- priority=priority, eta=eta, max_retries=max_retries)
26+ priority=priority, eta=eta, max_retries=max_retries, description=description)
27 job.user_id = self.session.uid
28 self.store(job)
29
30@@ -114,12 +114,14 @@
31 eta = kwargs.pop('eta', None)
32 model_name = kwargs.pop('model_name', None)
33 max_retries = kwargs.pop('max_retries', None)
34+ description = kwargs.pop('description', None)
35
36 return self.enqueue(func, model_name=model_name,
37 args=args, kwargs=kwargs,
38 priority=priority,
39 max_retries=max_retries,
40- eta=eta)
41+ eta=eta,
42+ description=description)
43
44 def exists(self, job_uuid):
45 """Returns if a job still exists in the storage."""
46@@ -229,7 +231,7 @@
47 eta = datetime.strptime(stored.eta, DEFAULT_SERVER_DATETIME_FORMAT)
48
49 job = Job(func=func_name, args=args, kwargs=kwargs,
50- priority=stored.priority, eta=eta, job_uuid=stored.uuid)
51+ priority=stored.priority, eta=eta, job_uuid=stored.uuid, description=stored.name)
52
53 if stored.date_created:
54 job.date_created = datetime.strptime(
55@@ -360,7 +362,7 @@
56
57 def __init__(self, func=None, model_name=None,
58 args=None, kwargs=None, priority=None,
59- eta=None, job_uuid=None, max_retries=None):
60+ eta=None, job_uuid=None, max_retries=None, description=None):
61 """ Create a Job
62
63 :param func: function to execute
64@@ -379,6 +381,8 @@
65 :param job_uuid: UUID of the job
66 :param max_retries: maximum number of retries before giving up and set
67 the job state to 'failed'. A value of 0 means infinite retries.
68+ :param description: human description of the job. If None, description
69+ is computed from the function doc or name
70 """
71 if args is None:
72 args = ()
73@@ -423,6 +427,7 @@
74 self.priority = DEFAULT_PRIORITY
75
76 self.date_created = datetime.now()
77+ self._description = description
78 self.date_enqueued = None
79 self.date_started = None
80 self.date_done = None
81@@ -483,7 +488,7 @@
82
83 @property
84 def description(self):
85- return self.func.__doc__ or 'Function %s' % self.func.__name__
86+ return self._description or self.func.__doc__ or 'Function %s' % self.func.__name__
87
88 @property
89 def uuid(self):
90@@ -597,7 +602,7 @@
91 Arguments and keyword arguments which will be given to the called
92 function once the job is executed. They should be ``pickle-able``.
93
94- There is 3 special and reserved keyword arguments that you can use:
95+ There is 4 special and reserved keyword arguments that you can use:
96
97 * priority: priority of the job, the smaller is the higher priority.
98 Default is 10.
99@@ -606,6 +611,10 @@
100 infinite retries. Default is 5.
101 * eta: the job can be executed only after this datetime
102 (or now + timedelta if a timedelta or integer is given)
103+
104+ * description : a human description of the job,
105+ intended to discriminate job instances
106+ (Default is the func.__doc__ or 'Function %s' % func.__name__)
107
108 Example:
109
110
111=== modified file 'connector/tests/test_job.py'
112--- connector/tests/test_job.py 2013-03-04 12:22:57 +0000
113+++ connector/tests/test_job.py 2013-11-18 14:29:22 +0000
114@@ -20,6 +20,8 @@
115
116
117 def task_a(session, model_name):
118+ """ Task description
119+ """
120 pass
121
122
123@@ -76,6 +78,22 @@
124 result = job.perform(self.session)
125 self.assertEqual(result, 'ok!')
126
127+ def test_description(self):
128+ """ If no description is given to the job, it
129+ should be computed from the function
130+ """
131+ # if a doctstring is defined for the function
132+ # it's used as description
133+ job_a = Job(func=task_a)
134+ self.assertEqual(job_a.description, task_a.__doc__)
135+ # if no docstring, the description is computed
136+ job_b = Job(func=task_b)
137+ self.assertEqual(job_b.description, "Function task_b")
138+ # case when we explicitly specify the description
139+ description = "My description"
140+ job_a = Job(func=task_a, description=description)
141+ self.assertEqual(job_a.description, description)
142+
143 def test_retryable_error(self):
144 job = Job(func=retryable_error_task,
145 max_retries=3)
146@@ -112,7 +130,8 @@
147 args=('o', 'k'),
148 kwargs={'c': '!'},
149 priority=15,
150- eta=eta)
151+ eta=eta,
152+ description="My description")
153 job.user_id = 1
154 storage = OpenERPJobStorage(self.session)
155 storage.store(job)