On Wed, Mar 19, 2014 at 10:15 AM, Michele Tartara <[email protected]>wrote:
> On Wed, Mar 19, 2014 at 9:55 AM, Hrvoje Ribicic <[email protected]> wrote: > > This patch refactors the current patch code to allow for multiple > > patches that can be applied, yet leaves only one for now. > > > > Signed-off-by: Hrvoje Ribicic <[email protected]> > > --- > > qa/qa_config.py | 46 +++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 39 insertions(+), 7 deletions(-) > > > > diff --git a/qa/qa_config.py b/qa/qa_config.py > > index 92f9584..4f93367 100644 > > --- a/qa/qa_config.py > > +++ b/qa/qa_config.py > > @@ -42,7 +42,8 @@ _ENABLED_DISK_TEMPLATES_KEY = "enabled-disk-templates" > > > > # The path of an optional JSON Patch file (as per RFC6902) that > modifies QA's > > # configuration. > > -_PATCH_JSON = os.path.join(os.path.dirname(__file__), "qa-patch.json") > > +_QA_BASE_PATH = os.path.dirname(__file__) > > +_QA_DEFAULT_PATCH = "qa-patch.json" > > > > #: QA configuration (L{_QaConfig}) > > _config = None > > @@ -254,6 +255,37 @@ class _QaConfig(object): > > #: Cluster-wide run-time value of the exclusive storage flag > > self._exclusive_storage = None > > > > + @staticmethod > > + def LoadPatch(patch_dict, rel_path): > > + """ Loads a single patch. > > + > > + @type patch_dict: dict of string to dict > > + @param patch_dict: A dictionary storing patches by relative path. > > + @type rel_path: string > > + @param rel_path: The relative path to the patch, might or might not > exist. > > + > > + """ > > + try: > > + full_path = os.path.join(_QA_BASE_PATH, rel_path) > > + patch = serializer.LoadJson(utils.ReadFile(full_path)) > > + if patch: > > + patch_dict[rel_path] = patch > > + except IOError: > > + pass > > + > > + @staticmethod > > + def LoadPatches(): > > + """ Loads all patches that can be found in the current QA directory. > > This is not really true, as of this patch. If this is the supposed > final role of this function, it's ok to leave this docstring, but > please add at least a TODO saying this is temporarily not true. > > Hm, this message needs improvement in general. I will replace it with a better description which will apply to later patches as well. > > + > > + @rtype: dict of string to dict > > + @return: A dictionary of relative path to patch content, for > non-empty > > + patches. > > + > > + """ > > + patches = {} > > + _QaConfig.LoadPatch(patches, _QA_DEFAULT_PATCH) > > + return patches > > + > > @classmethod > > def Load(cls, filename): > > """Loads a configuration file and produces a configuration object. > > @@ -268,16 +300,16 @@ class _QaConfig(object): > > # Patch the document using JSON Patch (RFC6902) in file > _PATCH_JSON, if > > # available > > try: > > - patch = serializer.LoadJson(utils.ReadFile(_PATCH_JSON)) > > - if patch: > > + patches = _QaConfig.LoadPatches() > > + if patches: > > mod = __import__("jsonpatch", fromlist=[]) > > - data = mod.apply_patch(data, patch) > > + data = mod.apply_patch(data, patches[_QA_DEFAULT_PATCH]) > > except IOError: > > pass > > except ImportError: > > - raise qa_error.Error("If you want to use the QA JSON patching > feature," > > - " you need to install Python modules" > > - " 'jsonpatch' and 'jsonpointer'.") > > + raise qa_error.Error("For the QA JSON patching feature to work, > you " > > + "need to install Python modules 'jsonpatch' > and " > > + "'jsonpointer'.") > > I like the rephrasing of this error message, but please add a > reference to it in the commit message. > Ack. > > > > > result = cls(dict(map(_ConvertResources, > > data.items()))) # pylint: disable=E1103 > > -- > > 1.9.0.279.gdc9e3eb > > > > 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 > New commit message: Refactor current patching code * Refactors the current patch code to allow for multiple patches that can be applied, yet leaves only one for now. * Rewords the message shown to the user in case the modules needed for patching are missing. Interdiff: diff --git a/qa/qa_config.py b/qa/qa_config.py index 4f93367..4f66180 100644 --- a/qa/qa_config.py +++ b/qa/qa_config.py @@ -275,7 +275,7 @@ class _QaConfig(object): @staticmethod def LoadPatches(): - """ Loads all patches that can be found in the current QA directory. + """ Finds and loads all non-empty patches supported by the QA. @rtype: dict of string to dict @return: A dictionary of relative path to patch content, for non-empty
