Skip to content
Snippets Groups Projects

Add a base-class for a resolver, and start raising exceptions if problems are encountered

Merged Add a base-class for a resolver, and start raising exceptions if problems are encountered
pelson/cmmnbuild-dep-manager:fix_always_reresolve into master
2 unresolved threads
Merged Philip Elson requested to merge pelson/cmmnbuild-dep-manager:fix_always_reresolve into master
2 unresolved threads

Add a base-class for a resolver, and start raising exceptions if problems are encountered (rather than logging them).

Includes:

  • Creation of an ABC for a Resolver
  • Ability to isolate the cmmnbuild_dep_manager.Manager for testing, and added some tests
  • Resolution of !10 (comment 2911676)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Philip Elson
    Philip Elson @pelson
    Author Owner

    @mihostet - this fixes !10 (comment 2911676) in two ways:

    • change the order to ensure that the resolver is defined before doing any checking of what is installed
    • raising an exception if a problem is encountered, so that we don't accidentally hide problems like this in the future

    Also, I don't have permissions on this repository - please could you add me as a maintainer?

  • Philip Elson
    Philip Elson @pelson started a thread on the diff
cmmnbuild_dep_manager/cmmnbuild_dep_manager.py
174 175 javalibpath,
175 176 '-Xss2m', # Required for kernels patching CVE-2017-1000364
176 177 '-Xrs', # Needed for proper handling of KeyboardInterrupt
177 '-Djava.class.path={0}'.format(self.class_path(extra_jars))
178 '-Djava.class.path={0}'.format(self.class_path(extra_jars)),
178 179 )
179 java_version = jpype.java.lang.System.getProperty('java.version')
180 java_version = str(jpype.java.lang.System.getProperty('java.version'))
  • Michi Hostettler
    Michi Hostettler @mihostet
    Owner

    Thanks a lot for proposing this. Fine for me. Did you test it on the well-known "real-world" examples (pyjapc, pjlsa, pytimber) as well?

    Just to be sure that not catching exceptions on module import does not introduce a regression - I could imagine that the modules might throw on import if JARs are still missing. On the other hand, we should have an internal catch in the module...

  • Philip Elson
    Philip Elson @pelson started a thread on the diff
    • cmmnbuild_dep_manager/cmmnbuild_dep_manager.py
    371 372 def is_installed(self, name, version=None):
    372 373 '''Check if module is installed'''
    373 374 modules = self._load_modules()
    374 if name in modules.keys():
    375 try:
    376 if version is None:
    377 version, _ = self._find_module_info(name)
    378 if modules[name] == version:
    379 return True
    380 except:
    381 pass
    375 if name in modules:
    376 if version is None:
    377 version, _ = self._find_module_info(name)
    • Philip Elson
      Philip Elson @pelson
      Author Owner

      This now has the possibility of raising if module info cannot be found. (this would be a strange case as we already know that the module is available from _load_modules().

    • Please register or sign in to reply
  • Michi Hostettler
    Michi Hostettler @mihostet
    Owner

    Btw, You should have Maintainer on that repo now. I'll be in a meeting for a bit. Please feel free to merge when you're ready.

  • Philip Elson merged

    merged

  • Philip Elson mentioned in commit 0397b52f

    mentioned in commit 0397b52f

  • Philip Elson mentioned in merge request !14 (closed)

    mentioned in merge request !14 (closed)

    • Please register or sign in to reply