Add dependencies in the prepare step instead of check_readiness. #2674
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using the prepare step we will be able to iterate over multiple build-dependencies such as multiple Python versions. This also removes the dependencies field from the Toolchain class which handles them completely functionally now. The EasyBlock class now has self.dependencies to remember them for outputting in modulefiles.
also use tcdeps instead of deps where appropriate for _load_modules()
Now that prepare_step() takes care of dependencies they now no longer fail.
boegel
requested changes
Nov 28, 2018
This avoids needing to worry about prepare returning a value, and easyblocks relying on it.
Contributor
Author
|
ping @boegel ? |
Member
|
@bartoldeman The problem with the tests is "fixed" in #2678, I'll retrigger the tests for this PR... |
boegel
requested changes
Dec 5, 2018
| # do all dependencies have a toolchain version? | ||
| if deps is None: | ||
| deps = [] | ||
| tcdeps = self._check_dependencies(deps) |
Member
There was a problem hiding this comment.
@bartoldeman Why not make this self.dependencies = self._check_dependencies(deps)?
Then there's no need to change _load_modules, _load_dependencies_modules and _add_dependency_variables to pass down tcdeps, they can just keep using self.dependencies like they were before?
It's a bit silly to pass them around if we have self.dependencies anyway (which we can't remove since easyblocks may be using it)...
add back Toolchain.add_dependencies as deprecated method
boegel
approved these changes
Dec 7, 2018
Member
|
Thanks @bartoldeman! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using the prepare step we will be able to iterate over multiple
build-dependencies such as multiple Python versions.
This also removes the dependencies field from the Toolchain class whichhandles them completely functionally now. The EasyBlock class now has
self.dependencies to remember them for outputting in modulefiles.