easybuild-easyblocks
easybuild-easyblocks copied to clipboard
Enable pip check by default
Support for running pip check at the end of the installation of a Python Package was recently added after a serious issue was detected in TensorFlow where mismatching dependencies led to hard-to-detect crashs.
To protect from further problems like that costing lot's of hours and hence money I'd suggest to enable the pip-check by default (not disable as for now)
Proposed solution:
Short term make the enable option a mandatory check in the CI for new or updated ECs, similar to e.g. https over http checks. This way all new or updated ECs already work with pip check (or need to be fixed)
On a decent system (build power and storage) grep for all Python packages, and reinstall them as a baseline to verify they still work. Then switch the default to enable pip check, reinstall the Python packages and fix detected issues for newish ECs. Older ones (and/or ones that don't seem worth maintaining) can be left broken until someone complains, have pip check manually disabled or be removed.
Ccing @boegel
Requiring sanity_pip_check = True in new/updated easyconfigs in PRs certainly makes perfect sense, and that's a pretty easy change to make in test/easyconfigs/easyconfigs.py.
Switching to having sanity_pip_check enabled by default is less easy though, even if we go through the (time-consuming) effort of checking all current easyconfig files for Python packages.
First of all, the latter is not as straightforward as just checking easyconfigs that use PythonPackage or PythonBundle, since some custom easyblocks derive from those (TensorFlow is an example there).
Second, we do not control all easyconfig files out there, just the ones in the central repository that are shipped with EasyBuild.
If enabling pip check by default makes installations that (seemingly) worked fine before fail with an update of EasyBuild, that's a regression (which we try hard to avoid). You could argue the installation is broken anyway if pip check fails, but that's not entirely correct, since the software may only actually need the missing dependency for some obscure feature...
Finally, we're quite new to pip check ourselves, and we may run into surprises with it (like #1877 for example...).
So, I'm all for required a gradual switch to enabling pip check, but it should done with care (so switching to having it enabled by default should only be done with a new major version of EasyBuild).
Requiring sanity_pip_check = True in new/updated easyconfigs in PRs certainly makes perfect sense, and that's a pretty easy change to make in test/easyconfigs/easyconfigs.py.
Great! Could you do that then? I'm not sure how to actually implement this given you'd need to check it in multiple places (exts, global, default opts)
So, I'm all for required a gradual switch to enabling pip check, but it should done with care (so switching to having it enabled by default should only be done with a new major version of EasyBuild).
Well yes. How about running it always, but downgrade to a warning which is always shown when sanity_pip_check is disabled? So users installing something see a nice warning coming up and can fix that or open issues? This allows to handle that "not as straightforward" process :)
Requiring sanity_pip_check = True in new/updated easyconfigs in PRs certainly makes perfect sense, and that's a pretty easy change to make in test/easyconfigs/easyconfigs.py.
Great! Could you do that then? I'm not sure how to actually implement this given you'd need to check it in multiple places (exts, global, default opts)
done, see https://github.com/easybuilders/easybuild-easyconfigs/pull/9516
So, I'm all for required a gradual switch to enabling pip check, but it should done with care (so switching to having it enabled by default should only be done with a new major version of EasyBuild).
Well yes. How about running it always, but downgrade to a warning which is always shown when
sanity_pip_checkis disabled? So users installing something see a nice warning coming up and can fix that or open issues? This allows to handle that "not as straightforward" process :)
That makes sense, I see no harm in always running pip check (unless it's hard disabled?) and producing a big fat warning if it failed, especially since the output is pretty concise & clear.
It should be fairly easy to change that in the PythonPackage easyblock...
What's the status of this? Could this be made the default soon or added to the to-be-changed defaults for EB 5?
This shouldn't be done before EasyBuild 5.0, I think... So I tagged it with the 5.0 milestone.