Hi David
Unless noted otherwise, changes have been made in the interdiff at the end.
2010/5/10 David Knowles <dknow...@google.com>:
> On Mon, May 10, 2010 at 11:30 AM, Michael Hanselmann <han...@google.com>
> wrote:
>> --- /dev/null
>> +++ b/lib/rapi/client_utils.py
>> +class RapiJobPollCb(cli.JobPollCbBase):
>
> What's "Cb"? Is that a callback? If so, it would be much more clear to call
> it "callback".
It's a callback and back when we implemented mcpu.OpExecCbBase, we
decided that short names are better.
>> + def __init__(self, cl):
>> + """Initializes this class.
>> +
>> + �...@param cl: RAPI client instance
>
> A more descriptive variable name would be nice.
“cl” is Ganeti convention for clients. See cli.py.
>> + cli.JobPollCbBase.__init__(self)
>> +
>> + self.cl = cl
>> +
>> + def WaitForJobChangeOnce(self, job_id, fields,
>> + prev_job_info, prev_log_serial):
>> + """Waits for changes on a job.
>
> This doesn't completely describe what this method does. It waits for a
> change, then what?
> Also, missing a @return doctag.
“WaitForJobChange[Once]” is, like “SubmitJob” or “CancelJob”, a
forwarded function from the job queue (jqueue.py). Since pydoc doesn't
have support for copying another functions'd docstring, we depend on
the reader to have some context.
>> + def QueryJobs(self, job_ids, fields):
>> + """Returns the selected fields for the selected job IDs.
>
> s/selected/given/g
> Also, missing @param and @return tags.
This is an overridden method from cli.JobPollCbBase. epydoc uses its
docstring if there's non in this subclass, but then we have no
docstring at all. And copying code or documentation sooner or later
leads to outdated versions. Copied from cli.JobPollCbBase.
Interdiff:
--- a/lib/rapi/client_utils.py
+++ b/lib/rapi/client_utils.py
@@ -64,11 +64,16 @@ class RapiJobPollCb(cli.JobPollCbBase):
return (result["job_info"], result["log_entries"])
def QueryJobs(self, job_ids, fields):
- """Returns the selected fields for the selected job IDs.
+ """Returns the given fields for the selected job IDs.
+
+ @type job_ids: list of numbers
+ @param job_ids: Job IDs
+ @type fields: list of strings
+ @param fields: Fields
"""
if len(job_ids) != 1:
- raise errors.ProgrammerError("Only one job supported at this time")
+ raise NotImplementedError("Only one job supported at this time")
try:
result = self.cl.GetJobStatus(job_ids[0])
@@ -87,7 +92,8 @@ def PollJob(rapi_client, job_id, reporter):
@param rapi_client: RAPI client instance
@type job_id: number
@param job_id: Job ID
- @param reporter: PollJob reporter
+ @type reporter: L{cli.JobPollReportCbBase}
+ @param reporter: PollJob reporter instance
"""
return cli.GenericPollJob(job_id, RapiJobPollCb(rapi_client), reporter)
