On Mon, May 10, 2010 at 11:30 AM, Michael Hanselmann <han...@google.com>wrote:
> The RAPI client module shouldn't depend on any Ganeti module, yet it's
> useful to have some Ganeti-specific code, like a PollJob function for
> RAPI.
> ---
> Makefile.am | 1 +
> lib/rapi/client_utils.py | 93
> ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 94 insertions(+), 0 deletions(-)
> create mode 100644 lib/rapi/client_utils.py
>
> diff --git a/Makefile.am b/Makefile.am
> index e0b5c2e..ddc0bdf 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -126,6 +126,7 @@ rapi_PYTHON = \
> lib/rapi/__init__.py \
> lib/rapi/baserlib.py \
> lib/rapi/client.py \
> + lib/rapi/client_utils.py \
> lib/rapi/connector.py \
> lib/rapi/rlib2.py
>
> diff --git a/lib/rapi/client_utils.py b/lib/rapi/client_utils.py
> new file mode 100644
> index 0000000..3431843
> --- /dev/null
> +++ b/lib/rapi/client_utils.py
> @@ -0,0 +1,93 @@
> +#
> +#
> +
> +# Copyright (C) 2010 Google Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +# General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +# 02110-1301, USA.
> +
> +
> +"""RAPI client utilities.
> +
> +"""
> +
> +from ganeti import errors
> +from ganeti import constants
> +from ganeti import cli
> +
> +from ganeti.rapi import client
> +
> +# Local constant to avoid importing ganeti.http
> +HTTP_NOT_FOUND = 404
> +
> +
> +class RapiJobPollCb(cli.JobPollCbBase):
>
What's "Cb"? Is that a callback? If so, it would be much more clear to call
it "callback".
> + def __init__(self, cl):
> + """Initializes this class.
> +
> + @param cl: RAPI client instance
>
A more descriptive variable name would be nice.
> +
> + """
> + 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.
> +
> + """
> + try:
> + result = self.cl.WaitForJobChange(job_id, fields,
> + prev_job_info, prev_log_serial)
> + except client.GanetiApiError, err:
> + if err.code == HTTP_NOT_FOUND:
> + return None
> +
> + raise
> +
> + if result is None:
> + return constants.JOB_NOTCHANGED
> +
> + return (result["job_info"], result["log_entries"])
> +
> + 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.
> +
> + """
> + if len(job_ids) != 1:
> + raise errors.ProgrammerError("Only one job supported at this time")
>
That's not really a ProgrammerError. A plural argument name implies
multiple items. This is more of a NotImplementedError.
> +
> + try:
> + result = self.cl.GetJobStatus(job_ids[0])
> + except client.GanetiApiError, err:
> + if err.code == HTTP_NOT_FOUND:
> + return [None]
> +
> + raise
> +
> + return [[result[name] for name in fields], ]
> +
> +
> +def PollJob(rapi_client, job_id, reporter):
> + """Function to poll for the result of a job.
> +
> + @param rapi_client: RAPI client instance
> + @type job_id: number
> + @param job_id: Job ID
> + @param reporter: PollJob reporter
>
I don't know what a reporter is. Please give this a type and a more
descriptive description.
> +
> + """
> + return cli.GenericPollJob(job_id, RapiJobPollCb(rapi_client), reporter)
> --
> 1.7.0.4
>
>
