#94 summarizer: better handling of local yaml files
Closed 4 months ago by nphilipp. Opened 4 months ago by rdossant.
modularity/ rdossant/fedmod summarize-libmmd-fix  into  master

file modified
+39 -10

@@ -88,14 +88,15 @@ 

  

  

  def _parse_mmd(mmd_index, profiles, dstreams, dprofiles, deps):

+     """

+     Parse module repodata information. This repodata is provided by a tool

+     (e.g MBS), so we can assume the information is well-formed

+     """

+ 

      for index in mmd_index:

          module_name = index.get_name()

          for module in index.get_streams().values():

-             # local modulemd files might miss some info like context or

-             # version. So let's make sure nsvc is consistent

-             ctxt = module.get_context() or ''

-             nsvc = f'{module_name}:{module.get_stream()}:' \

-                    f'{module.get_version()}:{ctxt}'

+             nsvc = module.get_nsvc()

              plist = list(module.get_profiles().keys())

              profiles[nsvc] = sorted(set(profiles.get(nsvc, []) + plist))

  

@@ -116,6 +117,34 @@ 

              dprofiles[module_name][s] = pset.get()

  

  

+ def _parse_objs(objects, profiles, dstreams, dprofiles, deps):

+     """

+     Parse module metadata information. This metadata is probably provided by

+     packager-written yamls, so it might be missing information (like module

+     name or module defaults).

+     """

+ 

+     for obj in objects:

+         if isinstance(obj, Modulemd.Module):

+             # local modulemd files might miss some info like context or

+             # version. So let's make sure nsvc is consistent

+             ctxt = obj.get_context() or ''

+             nsvc = f'{obj.get_name()}:{obj.get_stream()}:' \

+                    f'{obj.get_version()}:{ctxt}'

+             profiles[nsvc] = sorted(obj.get_profiles().keys())

+             deplist = []

+             for dep in obj.get_dependencies():

+                 for m, s in dep.peek_requires().items():

+                     deplist.append(f'{m}:{",".join(s.get())}'

+                                    if len(s.get()) else m)

+             deps[nsvc] = sorted(deplist)

+         elif isinstance(obj, Modulemd.Defaults):

+             module_name = obj.peek_module_name()

+             dstreams[module_name] = obj.peek_default_stream()

+             for s, pset in obj.peek_profile_defaults().items():

+                 dprofiles[module_name][s] = pset.get()

+ 

+ 

  def summarize_modules(mfilter=None, yamls=None, urls=None, as_tree=False):

      """

      Load Modulemd objects from each yaml file in the `yamls` list or each url

@@ -138,11 +167,11 @@ 

      profiles, dstreams, dprofiles, deps = {}, {}, defaultdict(dict), {}

      if yamls:

          for yaml in yamls:

-             index, failures = Modulemd.index_from_file(yaml)

-             if len(failures) != 0:

-                 msgs = "\n".join((str(f.get_gerror()) for f in failures))

-                 raise ClickException(f"Could not read {yaml}: {msgs}")

-             _parse_mmd(index.values(), profiles, dstreams, dprofiles, deps)

+             try:

+                 objects = Modulemd.objects_from_file(yaml)

+             except gi.repository.GLib.GError:

+                 raise ClickException(f"Could not read {yaml}: No modules")

I'm wary about interpreting any raised GError as "no modules". How about:

    ...
    except gi.repository.GLib.GError as gerror:
        raise ClickException(f"Could not read {yaml}: {gerror}") from gerror
    ...

This would also mark the GError as the direct cause of the ClickException (raise ... from ...).

+             _parse_objs(objects, profiles, dstreams, dprofiles, deps)

      elif urls:

          for url in urls:

              with tempfile.TemporaryDirectory() as local_path:

When reading a packager-provided yaml module file, some optional
information like module name, module stream or module defaults can be
missing. These are usually filled out by the module build service from
the module's git repo. So to correctly handle local yamls, we cannot use
the same modulemd_index_from_file call for both local and repo yamls.

See https://github.com/fedora-modularity/libmodulemd/issues/125

Signed-off-by: Rafael dos Santos rdossant@redhat.com

Metadata Update from @nphilipp:
- Request assigned

4 months ago

I'm wary about interpreting any raised GError as "no modules". How about:

    ...
    except gi.repository.GLib.GError as gerror:
        raise ClickException(f"Could not read {yaml}: {gerror}") from gerror
    ...

This would also mark the GError as the direct cause of the ClickException (raise ... from ...).

Looks good to me otherwise. Let me know what you think about handling the GError exceptions on which I commented above.

I'm fine with that. Should I send an update PR or will you make the change yourself?

I'll do it. Thanks for all the contributions in the recent past!

Pull-Request has been closed by nphilipp

4 months ago
Metadata