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

Bug fix for binding JS_PIPELINE_PATH differences between run and pipelines #164

Merged
merged 4 commits into from Nov 3, 2023

Conversation

bryce-turner
Copy link
Member

This fixes the handling of JS_PIPELINE_PATH within the slurm_singularity backend, namely due to the fact that jetstream run does not set JS_PIPELINE_PATH at runtime unless --pipeline is defined. The slurm_singularity backend was assuming that this value was always populated and would attempt to bind it by default within singularity.

Now, we check to see if JS_PIPELINE_PATH is in the environment and handle the export accordingly. Additionally, this PR adds in some bash variable expansion as well as replacing JS_PIPELINE_PATH with __pipeline__.path when necessary (within functions such as md5). Variable expansion is via os.path.expandvars(), and since JS_PIPELINE_PATH doesn't exist in the environment until runtime, we pass the pipeline context to the md5 function.

Finally, this also has adjustments for finding containers/images that are already in the cache dir. We previously only found images if the digest was defined for the task. Technically digest is an optional, but recommended directive. So now we search the cache dir within the job submission before attempting to pull externally. Theoretically, we should always find the image in the cache, but it's possible the cache has been cleaned since the start of the project and we want to avoid failures in these cases if possible.

…his allows us to search for cached images without necessarily looking for the digest
…pretting $JS_PIPELINE_PATH and replace this with the context value since JS_PIPELINE_PATH isn't in the env until runtime
Copy link
Member

@PedalheadPHX PedalheadPHX left a comment

Choose a reason for hiding this comment

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

Bit of a reversion to a good idea and adding support for atypical usage

@PedalheadPHX PedalheadPHX merged commit e9dd199 into develop Nov 3, 2023
5 checks passed
@PedalheadPHX PedalheadPHX deleted the fix_pipeline_path_for_run_command branch November 3, 2023 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants