Add optional mounts - #83
MichaelStubbings wants to merge 13 commits into
Conversation
When defining a list of mounts, you can now specify an 'optional_mount'. These are checked in the apptainer_entrypoint to see if the path exists. If not, then a warning is sent to stderr and the mount is excluded from the apptainer mount list.
ptsOSL
left a comment
ptsOSL
left a comment
There was a problem hiding this comment.
I think the premise is good, just a few details to improve on
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
==========================================
- Coverage 99.48% 99.19% -0.30%
==========================================
Files 27 27
Lines 978 995 +17
==========================================
+ Hits 973 987 +14
- Misses 5 8 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
MichaelStubbings
changed the title
ptsOSL
left a comment
ptsOSL
left a comment
There was a problem hiding this comment.
Thats a nice and easy way to prioritise the local entrypoint mounts. I am happy with these changes. Is it worth adding a test to check that the local entrypoint mounts are prioritised?
| ] = [] | ||
|
|
||
| optional_mounts: Annotated[ | ||
| list[MountPoint], |
There was a problem hiding this comment.
Is it possible to change this from list to set without affecting the configuration? Or would this be backwards-incompatible?
| ] = EntrypointOptions() | ||
|
|
||
| @model_validator(mode="after") | ||
| def prioritise_entrypoint_mounts(self) -> "ApptainerApp": |
There was a problem hiding this comment.
How does this work if you have multiple entrypoints with different mounts?
There was a problem hiding this comment.
This does not work very well with multiple entrypoints. When an entrypoint is removed from mounts, it is removed for all entrypoints.
I will investigate a solution outside of the model_validator. Either in validate.py or app_builder.py
| # Options and arguments to pass to command | ||
| command_args="{{ command_args }}" | ||
|
|
||
| {% if optional_mounts != None %} |
There was a problem hiding this comment.
Have you tested this with empty optional_mounts? The pattern I used in modulefile is: {% if dependencies|length %}
There was a problem hiding this comment.
This worked when optional mounts was a part of mounts. Now it does not. I've updated to your format: 3c98cc1
| done | ||
|
|
||
| mounts="${mounts},$(IFS=','; echo "${validated_mounts[*]}")" | ||
| {% endif %} |
There was a problem hiding this comment.
Why is there no whitespace handling? The modulefile template has an example of this.
This branch has not been deployed
3 participants
Adds optional mounts.
When an optional mount is invalid, it will exclude it from the mount list so it does not cause an error. This handles all types of mounting syntax, such as:
e.g. Using the options:
We see the following output: