Patch subject should be: Allow node acquisition and group creation in QA
using `with`
Exactly 60 characters! ;)

On Mon, Nov 11, 2013 at 4:37 PM, Petr Pudlak <[email protected]> wrote:

> For acquiring nodes use `with AcquireManyNodes(num): ...`. The nodes will
> be
> released automatically


Shouldn't the use actually be something like: "with AcquiredManyNodes(num)
as nodes:"?


> .
> For creating a new group use `with NewGroupCtx() as group: ...`. The group
> will
> be removed automatically.
>
> Signed-off-by: Petr Pudlak <[email protected]>
> ---
>  qa/qa_config.py | 16 ++++++++++++++++
>  qa/qa_group.py  | 12 ++++++++++++
>  2 files changed, 28 insertions(+)
>
> diff --git a/qa/qa_config.py b/qa/qa_config.py
> index 97ccaa7..a67943b 100644
> --- a/qa/qa_config.py
> +++ b/qa/qa_config.py
> @@ -757,6 +757,22 @@ def AcquireNode(exclude=None, _cfg=None):
>    return sorted(nodes, key=_NodeSortKey)[0].Use()
>
>
> +class AcquiredManyNodes(object):
> +  """Returns the least used nodes for use with a `with` block
>

Maybe "Used in a with, acquires the least used nodes and releases them
afterwards."?
Just to be in line with the other Ctx object documentation.


> +
> +  """
> +  def __init__(self, num, exclude=None):
> +    self._num = num
> +    self._exclude = exclude
> +
> +  def __enter__(self):
> +    self._nodes = AcquireManyNodes(self._num, self._exclude)
> +    return self._nodes
> +
> +  def __exit__(self, exc_type, exc_value, exc_tb):
> +    ReleaseManyNodes(self._nodes)
> +
> +
>  def AcquireManyNodes(num, exclude=None):
>    """Return the least used nodes.
>
> diff --git a/qa/qa_group.py b/qa/qa_group.py
> index da88602..d4f36f3 100644
> --- a/qa/qa_group.py
> +++ b/qa/qa_group.py
> @@ -127,6 +127,18 @@ def TestGroupAddWithOptions():
>    AssertCommand(["gnt-group", "remove", group1])
>
>
> +class NewGroupCtx(object):
> +  """Creates a new group and disposes afterwards."""
>
s/disposes/disposes of it/


> +
> +  def __enter__(self):
> +    (self._group, ) = qa_utils.GetNonexistentGroups(1)
> +    AssertCommand(["gnt-group", "add", self._group])
> +    return self._group
> +
> +  def __exit__(self, exc_type, exc_val, exc_tb):
> +    AssertCommand(["gnt-group", "remove", self._group])
> +
> +
>  def _GetGroupIPolicy(groupname):
>    """Return the run-time values of the cluster-level instance policy.
>
> --
> 1.8.4.1
>
>
I have a slight problem with the naming of the classes. Although they are
both convenience constructs to be used with with, you use quite different
naming.

I have not used Python `with` constructs enough to have any naming
recommendations, but consistency would be desirable :)

Thanks,

Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to