Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for reverse dependencies of installed modules (WIP) #2783

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

migueldiascosta
Copy link
Member

No description provided.

easybuild/tools/options.py Outdated Show resolved Hide resolved
easybuild/tools/options.py Outdated Show resolved Hide resolved
easybuild/tools/docs.py Outdated Show resolved Hide resolved
easybuild/tools/docs.py Outdated Show resolved Hide resolved
easybuild/tools/docs.py Outdated Show resolved Hide resolved
"""
reverse_dependencies = {}

module_avail_cmd = "module -t avail 2>&1 | grep -v ':$'"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would this work with a hierarchical scheme (or any scheme that involved extending the module path)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That shouldn't be a problem when using modules_tool().available(), which spits out a list of full module names in a hierarchy (i.e. things like Core/GCC/6.4.0 rather than just GCC/6.4.0), so you'll see all modules regardless of where they are in the hierarchy...

available_modules, _ = run_cmd(module_avail_cmd)

for module in available_modules.splitlines():
dependencies, _ = run_cmd(module_purge_load_list_cmd % (module,module))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@migueldiascosta probably a good idea to use trace=False here, otherwise you get very amusing output when using eb --trace (which is default for me)

reverse_dependencies = {}

module_avail_cmd = "module -t avail 2>&1 | grep -v ':$'"
module_purge_load_list_cmd = "module purge; module load %s; module -t list 2>&1 | grep -v %s"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using purge is troublesome on some systems, and fairly expensive with Lmod...

How about just running this in a subshell, since we only care about the output, not the changes to the environment?

Or use ml show (which only gives direct deps though), that would also make it significantly faster...

@ocaisa
Copy link
Member

ocaisa commented Feb 22, 2019

I was thinking about this recently but with a very different approach, you can generate a full dependency graph for your installation tree with

eb -D --dep-graph=depgraph.dot $EASYBUILD_INSTALLPATH/software/*/*/easybuild/*.eb

but it only works with certain caveats

  • You use a fixed install dir naming scheme (or use find which is very expensive)
  • There's no mechanism to check that the software is actually available in the module tree (but it wouldn't be hard to build)
  • It doesn't take into account user installed modules (but could easily)
  • It is probably pretty slow

easybuild/tools/docs.py Outdated Show resolved Hide resolved
easybuild/tools/docs.py Outdated Show resolved Hide resolved
@ocaisa
Copy link
Member

ocaisa commented Feb 22, 2019

I was thinking about this recently but with a very different approach, you can generate a full dependency graph for your installation tree with

eb -D --dep-graph=depgraph.dot $EASYBUILD_INSTALLPATH/software/*/*/easybuild/*.eb

but it only works with certain caveats

  • You use a fixed install dir naming scheme (or use find which is very expensive)
  • There's no mechanism to check that the software is actually available in the module tree (but it wouldn't be hard to build)
  • It doesn't take into account user installed modules (but could easily)
  • It is probably pretty slow

I tried it for a software in a stage of JURECA, and it (currently) doesn't work, graphdot throws errors like

pygraph.classes.exceptions.AdditionError: Edge (Core/JUnit/4.12, Core/Java/1.8) already in digraph

@ocaisa
Copy link
Member

ocaisa commented Feb 22, 2019

Sorry for bombing your PR but with a small tweak to tools.py

            try:
               dgr.add_edge((spec['module'], dep))
            except Exception as e:
               if "already in digraph" in str(e):
                   continue
               else:
                  raise e

my method succeeds and I get

[ocais1@jrl01 temp]$ time eb --detect-loaded-modules=ignore --robot=./ --dep-graph=depgraph.dot *.eb
== temporary log file in case of crash /tmp/eb-ogaUb9/easybuild-YAPyiN.log
== resolving dependencies ...
Wrote dependency graph for 1238 easyconfigs to depgraph.dot

real	1m42.163s
user	1m26.528s
sys	0m10.211s

@boegel
Copy link
Member

boegel commented Feb 22, 2019

@ocaisa One of the design principles here, which we also briefly discussed during the user meeting, is that the existing module files present the current "state" of things, and that's really all the matters from a users perspective.

Your approach is interesting, but not with the feature we have in mind that will be implemented on top of this (i.e. eb --remove).

@ocaisa
Copy link
Member

ocaisa commented Feb 22, 2019

But it would be easy to prune out non-installed easyconfigs (of which there would be none unless you manually removed the module files, note I'm only talking about easyconfigs in the easybuild directory of installed software here) since you have this info from the functionality of -D.
I kind of see where you are coming from but my approach allows you to see where something fits in the global tree, and even visualise it, which is exactly what you need with --remove.

@migueldiascosta
Copy link
Member Author

@ocaisa bomb away :), this WIP PR was created for precisely for discussing the issue

My original approach a while back was also based on easyconfigs in the robot path, and I still think there's a use case for that, but since then I came to agree with @boegel that for uninstall purposes the installed modules are the source of truth from the perspective of the users. For instance, there may be modules that were not created by easybuild but depend on modules created by easybuild, and we should know about those before removing a module created by easybuild, or even modules may have been modified after being created by easybuild...

Maybe there's a case for having both "easyconfig reverse dependencies", e.g. for removing easyconfigs from a repository, and "module reverse dependencies", e.g. for removing installed software?

As to this particular implementation of "module reverse dependencies", it's very slow, I'll look into using module show instead of module load + list to speed it up

…m ModuleGenerator and (modified, to optionally use MODULEPATH instead of calling module show, if using full module names) modulefile_path from ModuleTool
@migueldiascosta
Copy link
Member Author

@boegel by caching dependencies and, crucially, using $MODULEPATH + full module name to get the modulefile path instead of using module show, which is the default behaviour, this is now pretty fast (dependencies of ~2000 modules in ~20s, on pretty old and slow system and filesystem), the question is if it is correct in all cases, particularly different module tools and naming schemes...

@easybuilders easybuilders deleted a comment from boegelbot Mar 1, 2019
easybuild/tools/docs.py Outdated Show resolved Hide resolved
easybuild/tools/docs.py Outdated Show resolved Hide resolved
easybuild/tools/docs.py Outdated Show resolved Hide resolved
easybuild/tools/module_generator.py Outdated Show resolved Hide resolved
easybuild/tools/module_generator.py Outdated Show resolved Hide resolved
if mod_name not in alldeps:
alldeps[mod_name] = []

mod_filepath = modtool.modulefile_path(mod_name, full_module_names=full_module_names)
modtxt = read_file(mod_filepath)
loadregex = module_load_regex(mod_filepath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@migueldiascosta What about depends_on and prereq?

@boegelbot
Copy link

Travis test report: 1/8 runs failed - see https://travis-ci.org/easybuilders/easybuild-framework/builds/502458945

Only showing partial log for 1st failed test suite run 2867.7;
full log at https://travis-ci.org/easybuilders/easybuild-framework/jobs/502458954

...
{'maintainer': 'Stijn De Weirdt;Andy Georges;Jens Timmerman', 'install_requires': ['setuptools', 'mock'], 'setup_requires': ['setuptools'], 'maintainer_email': 'stijn.deweirdt@ugent.be, andy.georges@ugent.be, jens.timmerman@ugent.be', 'long_description': 'Description\n===========\nvsc-install provides shared setuptools functions and classes for python libraries developed by UGent\'s HPC group\n\nCommon pitfalls\n=========\nbdist_rpm will fail if your install_requires = \'setuptools\' because it will fail to find a setuptools rpm.\n```\nexport VSC_RPM_PYTHON=1\n```\nwill make sure the `python-` prefix is added to the packages in install_requires for building RPM\'s so python-setuptools will be used.\n\nAdd tests\n=========\n\nTest are python modules in the `test` directory which have subclass of `TestCase`\nand at least one method that has a name starting with `test_`\n\nYou are advised to use\n```python\nfrom vsc.install.testing import TestCase\n```\n(instead of basic `TestCase` from `unittest`).\n\nAnd any `__main__` or `suite()` is not needed (anymore).\n\nInitialise the test directory with\n\n```bash\nmkdir -p test\necho \'\' > test/__init__.py\necho \'from vsc.install.commontest import CommonTest\' > test/00-import.py\n```\n\nWhen the tests are run, `test`, `lib` and `bin` (if relevant) are added to `sys.path`,\nso no need to do so in the tets modules.\n\nRun tests\n=========\n\n```bash\npython setup.py test\n```\n\nFilter tests with `-F` (test module names) and `-f` (test method names)\n\nSee also\n\n```bash\npython setup.py test --help\n```\n\nIn case following error occurs, it means there is a test module `XYZ` that cannot be imported.\n\n```txt\nFile "setup.py", line 499, in loadTestsFromModule\n    testsuites = ScanningLoader.loadTestsFromModule(self, module)\nFile "build/bdist.linux-x86_64/egg/setuptools/command/test.py", line 37, in loadTestsFromModule\nFile "/usr/lib64/python2.7/unittest/loader.py", line 100, in loadTestsFromName\n    parent, obj = obj, getattr(obj, part)\nAttributeError: \'module\' object has no attribute \'XYZ\'\n```\n\nYou can try get the actual import error for fixing the issue with\n```bash\npython -c \'import sys;sys.path.insert(0, "test");import XYZ;\'\n```\n\nFix failing tests\n=================\n\n* Missing / incorrect `LICENSE`\n\n * Copy the appropirate license file under `known_licenses` in the project directory and name the file `LICENSE`\n\n* Missing `README.md`\n\n * Create a `README.md` file with at least a `Description` section\n\n* Fix license headers as described in https://github.com/hpcugent/vsc-install/blob/master/lib/vsc/install/headers.py\n\n  ```\n  cd <project dir with .git folder>\n  REPO_BASE_DIR=$PWD python -m vsc.install.headers path/to/file script_or_not\n  ```\n\n  Fix them all at once using find\n\n  ```\n  find ./{lib,test} -type f -name \'*.py\' | REPO_BASE_DIR=$PWD xargs -I \'{}\' python -m vsc.install.headers \'{}\'\n  find ./bin -type f -name \'*.py\' | REPO_BASE_DIR=$PWD xargs -I \'{}\' python -m vsc.install.headers \'{}\' 1\n  ```\n\n  Do not forget to check the diff.\n  Modules/scripts without docstring (or magic comment \'### END OF HEADER\') (incl. test modules)\n  will get correct header appended to existing one. Add a docstring (or magic comment) to resolve this.\n* Python scripts (i.e. with a python shebang and installed as scripts in setup) have to use `#!/usr/bin/env python` as shebang\n* Remove any `build_rpms_settings.sh` leftovers\n* The `TARGET` dict in `setup.py` should be minimal unless you really know what you are doing (i.e. if it is truly different from defaults)\n\n * Remove `name`, `scripts`, ...\n\n* `Exception: vsc namespace packages do not allow non-shared namespace`\n\n * Add to the `__init__.py`\n\n ```python\n """\n Allow other packages to extend this namespace, zip safe setuptools style\n """\n import pkg_resources\n pkg_resources.declare_namespace(__name__)\n ```\n\n\nbare-except\n-----------\n```python\ntry:\n   # something\nexcept:\n```\nThis is bad, because this except will also catch sys.exit() or Keyboardinterupts, something you\ntypically do not want, if you catch these the program will be in a weird state and then continue on,\nwhilst the person who just pressed ctrl+c is wondering what is going on and why it is not stopping.\n\nso at the very least make this\nexcept Exception (which doesn\'t catch sys.exit and KeyboardInterupt)\nand it would be appreciated if you could actually figure out what exceptions to expect and only catch those\nand let your program crash if something you did not intend happens\nbecause it helps developers catch weird errors on their side better.\n\nif you do something like\n```python\ntry:\n    open(int(somestring)).write(\'important data\')\nexcept Exception:\n    pass # if somestring is not an integer, we didn\'t need to write anyway, but otherwise we do\n```\nbecause you know sometimes this string does not contain an integer, so the int() call can fail\nyou should really only catch ValueError, because this will also fail when your disk is full, or you don\'t have permissions\nor xxx other reasons, and the important data will not be written out and nobody will notice anything!\n\n\n\nif not \'a\' in somelist -> if \'a\' not in somelist\n-------------------------------------------------\n\nthis isn\'t that big of a deal, but if everyone is consistent it\'s less likely to introduce bugs when a not is added or removed where it didn\'t need to.\nAlso helps code review, not in reads better, like english.\n\n\narguments-differ\n-----------------\n\nthis will give you errors if you override a function of a superclass but don\'t use the same amount of arguments,\nusing less will surely give you errors, so the linter catches this for you now\n\nunused-argument\n-----------------\nif you have a function definition witch accepts an argument that is never used in the function body this will now give an error.\nclean up your function definition, or fix the error where you actually do need to take this argument into account\n\nunused-variable\n----------------\ndefining a variable and then not using it anymore smells bad, why did you do that?\nsometimes you do things like\n```python\nout, exit_code = run_command(something)\n```\nbut you are not interested in the out, only in the exit code,\nin this case, write\n```python\n_, exit_code = run_command(something)\n```\n\nusing _ as a variable name lets pylint and other readers know you do not intend to use that output in the first place.\n\n\nreimported\n-------------\nwhen you re import a name somewhere else,\nusually this is just an import to much, or 2 imports with the same name, pay attention.\n```python\nimport six\nfrom django import six\n```\n=>\n```python\nimport six\nfrom django import six as django_six\n```\n\nredefinition of unused name\n----------------------------\nthis usually also points to something you did not expect\n```python\nfrom vsc.accountpageclient import VscGroup\n<snip>\n\nclass VscGroup(object):\n    pass\n```\n\n=> do you need the import? use import as\ndid you mean to use the same name? ...\n\nunpacking-in-except / redefine-in-handler\n-----------------------------------------\n\nMultiple exception have to be grouped in a tuple like\n\n```python\n    ...\nexcept (ExceptionOne, ExceptionTwo) ...\n    ...\n```\n\n(espcially when used like `except A, B:` which should be `except (A, B):`.\n\nturning off these errors\n-------------------------\n\nIf in any of these cases you think: yes, I really needed to do this,\nI\'m monkeypatching things, I\'m adding extra functionality that does indeed have an extra(default) paramenter, etc, etc\nyou can let pylint know to ignore this error in this one specific block of code\nby adding e.g. the comment `# pylint: disable=<name or numeric id of pylint code>`\n\n```python\nclass Something(object):\n    def dosomething(self, some, thing):\n        # do something\n\nclass MyFancyThing(SomeThing):\n    # pylint: disable=arguments-differ\n    def dosomething(self, some, thing, fancy=None):\n         # do something fancy\n```\n\nFull list with all codes is available at http://pylint-messages.wikidot.com/all-codes\n', 'tests_require': [], 'package_dir': {'': 'lib'}, 'author': 'Stijn De Weirdt;Andy Georges;Jens Timmerman', 'author_email': 'stijn.deweirdt@ugent.be, andy.georges@ugent.be, jens.timmerman@ugent.be', 'download_url': '', 'version': '0.11.6', 'description': "vsc-install provides shared setuptools functions and classes for python libraries developed by UGent's HPC group", 'namespace_packages': ['vsc'], 'scripts': ['bin/python-noenv.sh', 'bin/python-stripped-env'], 'command_packages': ['vsc.install.shared_setup', 'shared_setup_dist_only', 'setuptools.command', 'distutils.command'], 'packages': ['vsc.install', 'vsc'], 'classifiers': ['License :: OSI Approved :: GNU Lesser General Public License v2 or later (LGPLv2+)'], 'dependency_links': [], 'name': 'vsc-install', 'license': 'LGPLv2+', 'url': 'https://github.com/hpcugent/vsc-install', 'cmdclass': {'bdist_rpm': <class __main__.vsc_bdist_rpm at 0x21a8110>, 'install_scripts': <class __main__.vsc_install_scripts at 0x21a8230>, 'test': <class __main__.VscTestCommand at 0x21a80b0>, 'vsc_release': <class __main__.vsc_release at 0x21a8410>, 'egg_info': <class __main__.vsc_egg_info at 0x21a82f0>, 'sdist': <class __main__.vsc_sdist at 0x21a83b0>}, 'test_suite': 'test'}
WARN: zip_safe flag not set; analyzing archive contents...
WARN: vsc.fancylogger: module references __file__
WARN: vsc.install.commontest: module references __path__
WARN: vsc.install.shared_setup: module references __file__
WARN: vsc.install.shared_setup: module references __path__
WARN: vsc.install.shared_setup: module MAY be using inspect.getsource
INFO: creating /tmp/easy_install-E8FMBG/vsc-base-2.8.4/.eggs/vsc_install-0.11.6-py2.6.egg
INFO: Extracting vsc_install-0.11.6-py2.6.egg to /tmp/easy_install-E8FMBG/vsc-base-2.8.4/.eggs
INFO: 
Installed /tmp/easy_install-E8FMBG/vsc-base-2.8.4/.eggs/vsc_install-0.11.6-py2.6.egg
WARN: zip_safe flag not set; analyzing archive contents...
WARN: vsc.utils.fancylogger: module MAY be using inspect.stack
WARN: vsc.utils.generaloption: module MAY be using inspect.stack
WARN: vsc.utils.exceptions: module MAY be using inspect.getouterframes
creating /home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/vsc_base-2.8.4-py2.6.egg
Extracting vsc_base-2.8.4-py2.6.egg to /home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages
Adding vsc-base 2.8.4 to easy-install.pth file
Installing logdaemon script to /home/travis/virtualenv/python2.6.9/bin
Installing startlogdaemon script to /home/travis/virtualenv/python2.6.9/bin
Installing optcomplete.bash script to /home/travis/virtualenv/python2.6.9/bin

Installed /home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/vsc_base-2.8.4-py2.6.egg
Processing dependencies for vsc-base
Searching for vsc-install>=0.10.2
Doing git clone from https://github.com/hpcugent/vsc-install.git to /tmp/easy_install-TcEZnZ/vsc-install.git
Best match: vsc-install 0.10.2
Processing vsc-install.git
Writing /tmp/easy_install-TcEZnZ/vsc-install.git/setup.cfg
Running setup.py -q bdist_egg --dist-dir /tmp/easy_install-TcEZnZ/vsc-install.git/egg-dist-tmp-e7z0mo
INFO: This is (based on) vsc.install.shared_setup 0.11.6
INFO: run_tests from base dir /tmp/easy_install-TcEZnZ/vsc-install.git (using executable /tmp/easy_install-TcEZnZ/vsc-install.git/setup.py)
INFO: initial packages list: ['vsc.install', 'vsc']
INFO: generated list: ['vsc.install', 'vsc']
INFO: generated packages list: ['vsc.install', 'vsc']
INFO: makesetupcfg set to True, (re)creating setup.cfg
INFO: found license /tmp/easy_install-TcEZnZ/vsc-install.git/LICENSE with md5sum 5f30f0716dfdd0d91eb439ebec522ec2
INFO: Found license name LGPLv2+ and classifier License :: OSI Approved :: GNU Lesser General Public License v2 or later (LGPLv2+)
INFO: setting license LGPLv2+
INFO: found match url https://github.com/hpcugent/vsc-install in /tmp/easy_install-TcEZnZ/vsc-install.git/.git/config
INFO: found match name vsc-install in /tmp/easy_install-TcEZnZ/vsc-install.git/.git/config
INFO: Removing None download_url
INFO: get_name_url returns {'url': 'https://github.com/hpcugent/vsc-install', 'name': 'vsc-install'}
INFO: using long_description vsc-install provides shared setuptools functions and classes for python libraries developed by UGent's HPC group
INFO: generated list: ['bin/python-noenv.sh', 'bin/python-stripped-env']
INFO: generated scripts list: ['bin/python-noenv.sh', 'bin/python-stripped-env']
INFO: no prospector support in py26 (or older)
{'maintainer': 'Stijn De Weirdt;Andy Georges;Jens Timmerman', 'install_requires': ['setuptools', 'mock'], 'setup_requires': ['setuptools'], 'maintainer_email': 'stijn.deweirdt@ugent.be, andy.georges@ugent.be, jens.timmerman@ugent.be', 'long_description': 'Description\n===========\nvsc-install provides shared setuptools functions and classes for python libraries developed by UGent\'s HPC group\n\nCommon pitfalls\n=========\nbdist_rpm will fail if your install_requires = \'setuptools\' because it will fail to find a setuptools rpm.\n```\nexport VSC_RPM_PYTHON=1\n```\nwill make sure the `python-` prefix is added to the packages in install_requires for building RPM\'s so python-setuptools will be used.\n\nAdd tests\n=========\n\nTest are python modules in the `test` directory which have subclass of `TestCase`\nand at least one method that has a name starting with `test_`\n\nYou are advised to use\n```python\nfrom vsc.install.testing import TestCase\n```\n(instead of basic `TestCase` from `unittest`).\n\nAnd any `__main__` or `suite()` is not needed (anymore).\n\nInitialise the test directory with\n\n```bash\nmkdir -p test\necho \'\' > test/__init__.py\necho \'from vsc.install.commontest import CommonTest\' > test/00-import.py\n```\n\nWhen the tests are run, `test`, `lib` and `bin` (if relevant) are added to `sys.path`,\nso no need to do so in the tets modules.\n\nRun tests\n=========\n\n```bash\npython setup.py test\n```\n\nFilter tests with `-F` (test module names) and `-f` (test method names)\n\nSee also\n\n```bash\npython setup.py test --help\n```\n\nIn case following error occurs, it means there is a test module `XYZ` that cannot be imported.\n\n```txt\nFile "setup.py", line 499, in loadTestsFromModule\n    testsuites = ScanningLoader.loadTestsFromModule(self, module)\nFile "build/bdist.linux-x86_64/egg/setuptools/command/test.py", line 37, in loadTestsFromModule\nFile "/usr/lib64/python2.7/unittest/loader.py", line 100, in loadTestsFromName\n    parent, obj = obj, getattr(obj, part)\nAttributeError: \'module\' object has no attribute \'XYZ\'\n```\n\nYou can try get the actual import error for fixing the issue with\n```bash\npython -c \'import sys;sys.path.insert(0, "test");import XYZ;\'\n```\n\nFix failing tests\n=================\n\n* Missing / incorrect `LICENSE`\n\n * Copy the appropirate license file under `known_licenses` in the project directory and name the file `LICENSE`\n\n* Missing `README.md`\n\n * Create a `README.md` file with at least a `Description` section\n\n* Fix license headers as described in https://github.com/hpcugent/vsc-install/blob/master/lib/vsc/install/headers.py\n\n  ```\n  cd <project dir with .git folder>\n  REPO_BASE_DIR=$PWD python -m vsc.install.headers path/to/file script_or_not\n  ```\n\n  Fix them all at once using find\n\n  ```\n  find ./{lib,test} -type f -name \'*.py\' | REPO_BASE_DIR=$PWD xargs -I \'{}\' python -m vsc.install.headers \'{}\'\n  find ./bin -type f -name \'*.py\' | REPO_BASE_DIR=$PWD xargs -I \'{}\' python -m vsc.install.headers \'{}\' 1\n  ```\n\n  Do not forget to check the diff.\n  Modules/scripts without docstring (or magic comment \'### END OF HEADER\') (incl. test modules)\n  will get correct header appended to existing one. Add a docstring (or magic comment) to resolve this.\n* Python scripts (i.e. with a python shebang and installed as scripts in setup) have to use `#!/usr/bin/env python` as shebang\n* Remove any `build_rpms_settings.sh` leftovers\n* The `TARGET` dict in `setup.py` should be minimal unless you really know what you are doing (i.e. if it is truly different from defaults)\n\n * Remove `name`, `scripts`, ...\n\n* `Exception: vsc namespace packages do not allow non-shared namespace`\n\n * Add to the `__init__.py`\n\n ```python\n """\n Allow other packages to extend this namespace, zip safe setuptools style\n """\n import pkg_resources\n pkg_resources.declare_namespace(__name__)\n ```\n\n\nbare-except\n-----------\n```python\ntry:\n   # something\nexcept:\n```\nThis is bad, because this except will also catch sys.exit() or Keyboardinterupts, something you\ntypically do not want, if you catch these the program will be in a weird state and then continue on,\nwhilst the person who just pressed ctrl+c is wondering what is going on and why it is not stopping.\n\nso at the very least make this\nexcept Exception (which doesn\'t catch sys.exit and KeyboardInterupt)\nand it would be appreciated if you could actually figure out what exceptions to expect and only catch those\nand let your program crash if something you did not intend happens\nbecause it helps developers catch weird errors on their side better.\n\nif you do something like\n```python\ntry:\n    open(int(somestring)).write(\'important data\')\nexcept Exception:\n    pass # if somestring is not an integer, we didn\'t need to write anyway, but otherwise we do\n```\nbecause you know sometimes this string does not contain an integer, so the int() call can fail\nyou should really only catch ValueError, because this will also fail when your disk is full, or you don\'t have permissions\nor xxx other reasons, and the important data will not be written out and nobody will notice anything!\n\n\n\nif not \'a\' in somelist -> if \'a\' not in somelist\n-------------------------------------------------\n\nthis isn\'t that big of a deal, but if everyone is consistent it\'s less likely to introduce bugs when a not is added or removed where it didn\'t need to.\nAlso helps code review, not in reads better, like english.\n\n\narguments-differ\n-----------------\n\nthis will give you errors if you override a function of a superclass but don\'t use the same amount of arguments,\nusing less will surely give you errors, so the linter catches this for you now\n\nunused-argument\n-----------------\nif you have a function definition witch accepts an argument that is never used in the function body this will now give an error.\nclean up your function definition, or fix the error where you actually do need to take this argument into account\n\nunused-variable\n----------------\ndefining a variable and then not using it anymore smells bad, why did you do that?\nsometimes you do things like\n```python\nout, exit_code = run_command(something)\n```\nbut you are not interested in the out, only in the exit code,\nin this case, write\n```python\n_, exit_code = run_command(something)\n```\n\nusing _ as a variable name lets pylint and other readers know you do not intend to use that output in the first place.\n\n\nreimported\n-------------\nwhen you re import a name somewhere else,\nusually this is just an import to much, or 2 imports with the same name, pay attention.\n```python\nimport six\nfrom django import six\n```\n=>\n```python\nimport six\nfrom django import six as django_six\n```\n\nredefinition of unused name\n----------------------------\nthis usually also points to something you did not expect\n```python\nfrom vsc.accountpageclient import VscGroup\n<snip>\n\nclass VscGroup(object):\n    pass\n```\n\n=> do you need the import? use import as\ndid you mean to use the same name? ...\n\nunpacking-in-except / redefine-in-handler\n-----------------------------------------\n\nMultiple exception have to be grouped in a tuple like\n\n```python\n    ...\nexcept (ExceptionOne, ExceptionTwo) ...\n    ...\n```\n\n(espcially when used like `except A, B:` which should be `except (A, B):`.\n\nturning off these errors\n-------------------------\n\nIf in any of these cases you think: yes, I really needed to do this,\nI\'m monkeypatching things, I\'m adding extra functionality that does indeed have an extra(default) paramenter, etc, etc\nyou can let pylint know to ignore this error in this one specific block of code\nby adding e.g. the comment `# pylint: disable=<name or numeric id of pylint code>`\n\n```python\nclass Something(object):\n    def dosomething(self, some, thing):\n        # do something\n\nclass MyFancyThing(SomeThing):\n    # pylint: disable=arguments-differ\n    def dosomething(self, some, thing, fancy=None):\n         # do something fancy\n```\n\nFull list with all codes is available at http://pylint-messages.wikidot.com/all-codes\n', 'tests_require': [], 'package_dir': {'': 'lib'}, 'author': 'Stijn De Weirdt;Andy Georges;Jens Timmerman', 'author_email': 'stijn.deweirdt@ugent.be, andy.georges@ugent.be, jens.timmerman@ugent.be', 'download_url': '', 'version': '0.11.6', 'description': "vsc-install provides shared setuptools functions and classes for python libraries developed by UGent's HPC group", 'namespace_packages': ['vsc'], 'scripts': ['bin/python-noenv.sh', 'bin/python-stripped-env'], 'command_packages': ['vsc.install.shared_setup', 'shared_setup_dist_only', 'setuptools.command', 'distutils.command'], 'packages': ['vsc.install', 'vsc'], 'classifiers': ['License :: OSI Approved :: GNU Lesser General Public License v2 or later (LGPLv2+)'], 'dependency_links': [], 'name': 'vsc-install', 'license': 'LGPLv2+', 'url': 'https://github.com/hpcugent/vsc-install', 'cmdclass': {'bdist_rpm': <class __main__.vsc_bdist_rpm at 0x26de2f0>, 'install_scripts': <class __main__.vsc_install_scripts at 0x26de7d0>, 'test': <class __main__.VscTestCommand at 0x26de590>, 'vsc_release': <class __main__.vsc_release at 0x26deb30>, 'egg_info': <class __main__.vsc_egg_info at 0x26de890>, 'sdist': <class __main__.vsc_sdist at 0x26de9b0>}, 'test_suite': 'test'}
WARN: zip_safe flag not set; analyzing archive contents...
WARN: vsc.fancylogger: module references __file__
WARN: vsc.install.commontest: module references __path__
WARN: vsc.install.shared_setup: module references __file__
WARN: vsc.install.shared_setup: module references __path__
WARN: vsc.install.shared_setup: module MAY be using inspect.getsource
creating /home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/vsc_install-0.11.6-py2.6.egg
Extracting vsc_install-0.11.6-py2.6.egg to /home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages
Adding vsc-install 0.11.6 to easy-install.pth file
Installing python-noenv script to /home/travis/virtualenv/python2.6.9/bin
Installing python-stripped-env script to /home/travis/virtualenv/python2.6.9/bin

Installed /home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/vsc_install-0.11.6-py2.6.egg
Finished processing dependencies for vsc-base
travis_time:end:1e584a0c:start=1551864043806702670,finish=1551864047030383808,duration=3223681138
�[0Ktravis_fold:end:install.1
�[0Ktravis_fold:start:install.2
�[0Ktravis_time:start:2fd37788
�[0K$ export INSTALL_DEP=$TRAVIS_BUILD_DIR/easybuild/scripts/install_eb_dep.sh
travis_time:end:2fd37788:start=1551864047034696625,finish=1551864047037358071,duration=2661446
�[0Ktravis_fold:end:install.2
�[0Ktravis_fold:start:install.3
�[0Ktravis_time:start:11050b39
�[0K$ if [ ! -z $ENV_MOD_VERSION ]; then source $INSTALL_DEP modules-${ENV_MOD_VERSION} $HOME; fi
Installing modules-4.1.4 @ /home/travis...
--2019-03-06 09:20:47--  http://prdownloads.sourceforge.net/modules/modules-4.1.4.tar.gz
Resolving prdownloads.sourceforge.net (prdownloads.sourceforge.net)... 216.105.38.13
Connecting to prdownloads.sourceforge.net (prdownloads.sourceforge.net)|216.105.38.13|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: /kLXVh/modules/modules-4.1.4.tar.gz [following]
--2019-03-06 09:20:47--  http://prdownloads.sourceforge.net/kLXVh/modules/modules-4.1.4.tar.gz
Connecting to prdownloads.sourceforge.net (prdownloads.sourceforge.net)|216.105.38.13|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: /modules/modules-4.1.4.tar.gz [following]
--2019-03-06 09:20:47--  http://prdownloads.sourceforge.net/modules/modules-4.1.4.tar.gz
Connecting to prdownloads.sourceforge.net (prdownloads.sourceforge.net)|216.105.38.13|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: /kLXVh/modules/modules-4.1.4.tar.gz [following]
--2019-03-06 09:20:47--  http://prdownloads.sourceforge.net/kLXVh/modules/modules-4.1.4.tar.gz
Connecting to prdownloads.sourceforge.net (prdownloads.sourceforge.net)|216.105.38.13|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: /modules/modules-4.1.4.tar.gz [following]
--2019-03-06 09:20:47--  http://prdownloads.sourceforge.net/modules/modules-4.1.4.tar.gz
Connecting to prdownloads.sourceforge.net (prdownloads.sourceforge.net)|216.105.38.13|:80... connected.
HTTP request sent, awaiting response... 302 Found
Location: /kLXVh/modules/modules-4.1.4.tar.gz [following]
--2019-03-06 09:20:47--  http://prdownloads.sourceforge.net/kLXVh/modules/modules-4.1.4.tar.gz
Connecting to prdownloads.sourceforge.net (prdownloads.sourceforge.net)|216.105.38.13|:80... connected.
HTTP request sent, awaiting response... 307 Temporary Redirect
Location: https://sourceforge.net/downloads [following]
--2019-03-06 09:20:47--  https://sourceforge.net/downloads
Resolving sourceforge.net (sourceforge.net)... 216.105.38.13
Connecting to sourceforge.net (sourceforge.net)|216.105.38.13|:443... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: https://sourceforge.net/publish [following]
--2019-03-06 09:20:47--  https://sourceforge.net/publish
Connecting to sourceforge.net (sourceforge.net)|216.105.38.13|:443... connected.
Unable to establish SSL connection.
/home/travis/build/easybuilders/easybuild-framework/easybuild/scripts/install_eb_dep.sh: line 71: cd: modules-4.1.4: No such file or directory

*bleep, bloop, I'm just a bot (boegelbot v20180813.01)*Please talk to my owner @boegel if you notice you me acting stupid),or submit a pull request to https://github.com/boegel/boegelbot fix the problem.

@migueldiascosta
Copy link
Member Author

This currently works for a flat module naming scheme, but not for one that extends the module path.

For HierarchicalMNS, it is possible to recurse through the hierarchy levels, keeping track of extended module paths, short and full module names for each dependency, but it's messy and even then I'm not sure it would work for other naming schemes...

@easybuilders easybuilders deleted a comment from boegelbot Apr 3, 2019
@boegel boegel modified the milestones: next release (3.9.1), 3.x May 14, 2019
@boegel boegel modified the milestones: 3.x, 4.x Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants