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 functionality for Spack module to take data runners #1496

Merged

Conversation

nick-stroud
Copy link
Collaborator

data_files are a subset of startup-script.runners. They can only be of type data and must have a destination of an absolute path. While this same functionality could be achieved by using startup script directly, it was decided that it would be logically cleaner to define all of the spack configs in a single module.

It was decided to not duplicate the validation that startup script does. This leaves some minor naming mismatch in error messages as validation will report that "runner cannot do such and such" instead of "data_files".

Tested manually: Upload file via data runner and then cat the file in commands

Submission Checklist

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #
  • Confirm that "make tests" passes all tests
  • Add or modify unit tests to cover code changes
  • Ensure that unit test coverage remains above 80%
  • Update all applicable documentation
  • Follow Cloud HPC Toolkit Contribution guidelines #

Copy link
Member

@tpdownes tpdownes left a comment

Choose a reason for hiding this comment

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

Effectively, I believe sources must be absolute until we support a mechanism for staging data into the deployment directory. It's a "Toolkit limit" and not a bar I expect this PR to clear. I like that it also content, at least for text files.

Please make the change I request and merge.

@tpdownes tpdownes assigned nick-stroud and unassigned tpdownes Jun 23, 2023
@nick-stroud nick-stroud enabled auto-merge June 23, 2023 20:57
@nick-stroud nick-stroud merged commit 896c4a1 into GoogleCloudPlatform:spack-redesign Jun 23, 2023
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.

3 participants