Skip to content

require that sanity_pip_check is enabled in new/changed easyconfigs#9516

Merged
smoors merged 1 commit intoeasybuilders:developfrom
boegel:require_sanity_check_pip
Dec 18, 2019
Merged

require that sanity_pip_check is enabled in new/changed easyconfigs#9516
smoors merged 1 commit intoeasybuilders:developfrom
boegel:require_sanity_check_pip

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Dec 15, 2019

Support for running pip check during the sanity check to ensure that all required Python packages are installed was added in EasyBuild v4.1.0 (see #1853).

We should start requiring it in contributed easyconfigs, since it can prevent shipping broken easyconfigs (see #9306 for example).

You can enable the running of pip check either via:

sanity_pip_check = True

or

exts_default_options = {'sanity_pip_check': True}

(it can also be enabled for individual extensions, but there's little point in doing that imho, so no need to cover that in the check here)

@Flamefire
Copy link
Copy Markdown
Contributor

Great! But we should also fix easybuilders/easybuild-easyblocks#1877 to avoid failures due to python using the user home.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Dec 17, 2019

easybuilders/easybuild-easyblocks#1877 is being fixed with easybuilders/easybuild-easyblocks#1891

@smoors This is good to go imho...

@boegel boegel force-pushed the require_sanity_check_pip branch from 45471df to 55ac2f7 Compare December 17, 2019 21:20
@Flamefire
Copy link
Copy Markdown
Contributor

Flamefire commented Dec 18, 2019

Thanks, LGTM. However I suggest to rework the assert messages (for pretty much all tests). The current one here is e.g. "sanity_pip_check is enabled in X". So if you see the test failing on travis this will be what is printed. The problem: The opposite is the case: The check failed because it is not enabled. This made me loose some time chasing for the failure because I couldn't understand the message. Better: "sanity_pip_check should be enabled in X" or "sanity_pip_check is not enabled in X"

@smoors
Copy link
Copy Markdown
Contributor

smoors commented Dec 18, 2019

@Flamefire I understand your confusion, but all other error messages are constructed in the same way.
So maybe file an issue on that?

Copy link
Copy Markdown
Contributor

@smoors smoors left a comment

Choose a reason for hiding this comment

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

lgtm

@Flamefire
Copy link
Copy Markdown
Contributor

Ok: #9542

@smoors
Copy link
Copy Markdown
Contributor

smoors commented Dec 18, 2019

Test report by @smoors
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
login2.cerberus.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) Gold 6126 CPU @ 2.60GHz, Python 2.7.5
See https://gist.github.com/c8ff78e72b7d37aa98ea8d20075fff7f for a full test report.

@smoors
Copy link
Copy Markdown
Contributor

smoors commented Dec 18, 2019

Going in, thanks @boegel!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants