On Wed, Mar 19, 2014 at 10:35 AM, Hrvoje Ribicic <[email protected]> wrote:
> My apologies for these two errors, they slipped past me.
>
> Thanks a lot for catching them, the interdiff is:
>
> diff --git a/qa/qa_config.py b/qa/qa_config.py
> index cff568d..2bf0ac6 100644
> --- a/qa/qa_config.py
> +++ b/qa/qa_config.py
> @@ -296,10 +296,8 @@ class _QaConfig(object):
>    def ApplyPatches(data, patch_module, patches):
>      """Applies any patches present, and returns the modified QA
> configuration.
>
> -    First, patches from the patch directory are applied. They are ordered
> -    alphabetically, unless there is an ``order`` file present - any patches
> -    listed within are applied in that order, and any remaining ones in
> -    alphabetical order again.
> +    First, patches from the patch directory are applied, ordered
> alphabetically
> +    by name.
>
>      @type data: dict (deserialized json)
>      @param data: The QA configuration
> @@ -336,7 +334,6 @@ class _QaConfig(object):
>      try:
>        patches = _QaConfig.LoadPatches()
>        if patches:
> -        print patches
>          mod = __import__("jsonpatch", fromlist=[])
>          data = _QaConfig.ApplyPatches(data, mod, patches)
>      except IOError:
>
>
>
> On Wed, Mar 19, 2014 at 10:19 AM, Michele Tartara <[email protected]>
> wrote:
>>
>> On Wed, Mar 19, 2014 at 9:55 AM, Hrvoje Ribicic <[email protected]> wrote:
>> > This patch allows support for multiple patches placed in the "patch"
>> > directory, which are executed in alphabetical order.
>> >
>> > Signed-off-by: Hrvoje Ribicic <[email protected]>
>> > ---
>> >  qa/qa_config.py | 39 +++++++++++++++++++++++++++++++++++++--
>> >  1 file changed, 37 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/qa/qa_config.py b/qa/qa_config.py
>> > index 4f93367..0340883 100644
>> > --- a/qa/qa_config.py
>> > +++ b/qa/qa_config.py
>> > @@ -40,10 +40,11 @@ _VCLUSTER_MASTER_KEY = "vcluster-master"
>> >  _VCLUSTER_BASEDIR_KEY = "vcluster-basedir"
>> >  _ENABLED_DISK_TEMPLATES_KEY = "enabled-disk-templates"
>> >
>> > -# The path of an optional JSON Patch file (as per RFC6902) that
>> > modifies QA's
>> > +# The constants related to JSON patching (as per RFC6902) that modifies
>> > QA's
>> >  # configuration.
>> >  _QA_BASE_PATH = os.path.dirname(__file__)
>> >  _QA_DEFAULT_PATCH = "qa-patch.json"
>> > +_QA_PATCH_DIR = "patch"
>> >
>> >  #: QA configuration (L{_QaConfig})
>> >  _config = None
>> > @@ -284,8 +285,41 @@ class _QaConfig(object):
>> >      """
>> >      patches = {}
>> >      _QaConfig.LoadPatch(patches, _QA_DEFAULT_PATCH)
>> > +    patch_dir_path = os.path.join(_QA_BASE_PATH, _QA_PATCH_DIR)
>> > +    if os.path.exists(patch_dir_path):
>> > +      for filename in os.listdir(patch_dir_path):
>> > +        if filename.endswith(".json"):
>> > +          _QaConfig.LoadPatch(patches, os.path.join(_QA_PATCH_DIR,
>> > filename))
>> >      return patches
>> >
>> > +  @staticmethod
>> > +  def ApplyPatches(data, patch_module, patches):
>> > +    """Applies any patches present, and returns the modified QA
>> > configuration.
>> > +
>> > +    First, patches from the patch directory are applied. They are
>> > ordered
>> > +    alphabetically, unless there is an ``order`` file present - any
>> > patches
>> > +    listed within are applied in that order, and any remaining ones in
>> > +    alphabetical order again.
>>
>> Either specify in this patch that there is no support for order yet,
>> but this will be the final state, or don't add any reference to it
>> until the patch where it will actually be implemented.
>>
>> > +
>> > +    @type data: dict (deserialized json)
>> > +    @param data: The QA configuration
>> > +    @type patch_module: module
>> > +    @param patch_module: The json patch module, loaded dynamically
>> > +    @type patches: dict of string to dict
>> > +    @param patches: The dictionary of patch path to content
>> > +
>> > +    @return: The modified configuration data.
>> > +
>> > +    """
>> > +    for patch in sorted(patches):
>> > +      if patch != _QA_DEFAULT_PATCH:
>> > +        data = patch_module.apply_patch(data, patches[patch])
>> > +
>> > +    if _QA_DEFAULT_PATCH in patches:
>> > +      data = patch_module.apply_patch(data, patches[_QA_DEFAULT_PATCH])
>> > +
>> > +    return data
>> > +
>> >    @classmethod
>> >    def Load(cls, filename):
>> >      """Loads a configuration file and produces a configuration object.
>> > @@ -302,8 +336,9 @@ class _QaConfig(object):
>> >      try:
>> >        patches = _QaConfig.LoadPatches()
>> >        if patches:
>> > +        print patches
>>
>> Looks like a residual print statement from testing. I guess it has to
>> be removed?
>>
>> >          mod = __import__("jsonpatch", fromlist=[])
>> > -        data = mod.apply_patch(data, patches[_QA_DEFAULT_PATCH])
>> > +        data = _QaConfig.ApplyPatches(data, mod, patches)
>> >      except IOError:
>> >        pass
>> >      except ImportError:
>> > --
>> > 1.9.0.279.gdc9e3eb
>> >
>>
>> Rest LGTM, thanks.
>>
>> Cheers,
>> Michele
>>
>> --
>> 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
>
>

LGTM, thanks.

Michele

-- 
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

Reply via email to