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

change 'eb' command to import easybuild to check if it's working #3995

Merged
merged 4 commits into from
Apr 22, 2022

Conversation

bartoldeman
Copy link
Contributor

@bartoldeman bartoldeman commented Apr 19, 2022

This is a much cheaper test than import easybuild.main and still
solves the same issue as #3610.

This reduces the wall time for eb --version from 1.35s to 0.73s
in our configuration.

This is a much cheaper test than "import easybuild.main" and still
solves the same issue as easybuilders#3610.

This reduces the wall time for "eb --version" from 1.35s to 0.73s
in our configuration.
@bartoldeman
Copy link
Contributor Author

See also #3180

eb Outdated Show resolved Hide resolved
eb Outdated Show resolved Hide resolved
eb Outdated Show resolved Hide resolved
Copy link
Contributor

@casparvl casparvl left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've installed from the feature branch, and tested timings. For me it was about 25% faster to run eb --version. I've also tested a basic installation (which worked fine) just as a sanity check, even though this PR shouldn't change anything that could affect installations - only the check for which python to use was touched.

To me, the change looks sane and should still return the correct Python, namely the one used to install EasyBuild.

@casparvl casparvl dismissed akesandgren’s stale review April 22, 2022 16:23

Requested changes were implemented

@casparvl casparvl merged commit c250be5 into easybuilders:develop Apr 22, 2022
@boegel
Copy link
Member

boegel commented Apr 27, 2022

This is actually not equivalent at all, since if somehow only the eb command and the easybuild-easyblocks package are available, import easybuild will also work, but easybuild.main (or anything else provided by framework) may not be, so I'm not sure this change is a good idea...

@boegel boegel changed the title Change 'eb' command to import easybuild to check if it's working. change 'eb' command to import easybuild to check if it's working Apr 27, 2022
@boegel
Copy link
Member

boegel commented Apr 27, 2022

An alternative could be to test the import of easybuild.framework, which is very light weight as well (doesn't import other things from easybuild namespace), and that's definitely a better check to verify that EasyBuild framework is available...

Follow-up PR at #3998

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.

4 participants