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 detecting auto-downloaded dependencies in PythonPackage easyblock #1377

Merged
merged 3 commits into from
May 17, 2018

Conversation

boegel
Copy link
Member

@boegel boegel commented Mar 6, 2018

@boegel boegel added this to the 3.6.0 milestone Mar 6, 2018
@@ -178,17 +178,18 @@ def extra_options(extra_vars=None):
if extra_vars is None:
extra_vars = {}
extra_vars.update({
'unpack_sources': [True, "Unpack sources prior to build/install", CUSTOM],
'buildcmd': ['build', "Command to pass to setup.py to build the extension", CUSTOM],
'download_dep_fail': [False, "Fail if downloaded dependencies are detected", CUSTOM],
Copy link
Member Author

Choose a reason for hiding this comment

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

Set to False for backward-compatibility, but we should consider switching this to True by default for EasyBuild v4.0.0.

Problem is that we'll need to fix quite a bit of easyconfigs...

We could try to enforce a policy where new easyconfigs for Python packages include download_dep_fail = True?

@boegel boegel requested a review from wpoely86 March 6, 2018 17:53
@boegel
Copy link
Member Author

boegel commented Apr 15, 2018

@wpoely86 please review?

@boegel boegel modified the milestones: 3.6.0, next release Apr 24, 2018
vanzod
vanzod previously approved these changes Apr 27, 2018
@boegel
Copy link
Member Author

boegel commented Apr 27, 2018

Following @akesandgren's suggestion during this week's conf call to enhance this such that it holds off reporting an error until all extensions have been installed (which helps in dealing with problems in one go), I think we should move the download check to the sanity_check_step.

We already postpone reporting sanity check failures for extensions (in general) until all extensions are installed, so we can ride on that for this too.

There's one downside though... There's currently no support for including a custom message to the error message that is produced when the sanity check fails.
That problem exists already (you need to go hunting in the log for what the actual problem is when an import check fails) but also using the PythonPackage sanity check for the download check will make that worse (since there will be multiple possible causes for the extension sanity check to fail).

Hence, I think we should make some changes to framework first to improve error reporting for failing sanity checks for extensions...

…Package generic easyblock + leverage general 'exts_download_dep_fail' easyconfig parameter
@boegel
Copy link
Member Author

boegel commented Apr 30, 2018

@akesandgren suggestion is now implemented, but it depends on two changes in EasyBuild framework (see updated description)

@boegel
Copy link
Member Author

boegel commented May 7, 2018

The framework PRs on which this depends have been merged, so this is good to go?

@vanzod
Copy link
Member

vanzod commented May 17, 2018

My tests did not show any issue. Merging

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.

2 participants