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

Remove script_dir state from channels #3714

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

benclifford
Copy link
Collaborator

This leaves channels as a stateless (and useless) object that can be removed in a subsequent PR. That removal will change/break a lot of config files so I want it to keep it separate from this attribute removal.

Changed Behaviour

Batch providers will now use their own script_dir attribute, rather than the channel script_dir attribute. Prior to PR #3688 these attributes were paths on different file systems: the submit side (for providers) and the batch job execution side (for channels). PR #3688 removed support for batch job commands to run somewhere that isn't the workflow submit side, and so since then, these two attributes have been roughly equivalent.

In some (obscure seeming cases) they might be different: if a channel is shared between DFKs, then the script_dir used by providers in one DFK will now no longer use the channel script dir set by the other DFK. This was probably a bug anyway but I am noting it because this PR isn't completely behaviour preserving.

Type of change

  • Code maintenance/cleanup

Copy link
Collaborator

@khk-globus khk-globus left a comment

Choose a reason for hiding this comment

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

This is quite a nice tidy-up. I like it a lot.

@khk-globus khk-globus added this pull request to the merge queue Dec 3, 2024
Merged via the queue into master with commit 76e5328 Dec 3, 2024
7 checks passed
@khk-globus khk-globus deleted the benc-3515-scriptdir branch December 3, 2024 17:16
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.

2 participants