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

File staging #450

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open

File staging #450

wants to merge 33 commits into from

Conversation

hategan
Copy link
Collaborator

@hategan hategan commented Feb 24, 2024

This PR adds file staging to some executors (local and the batch family).

  • It does so mostly through templates.
  • It addresses the status updates issues mentioned in Added file staging job-api-spec#164 using a UDP service that simple netcat commands (and we have to carefully test there because there might be multiple versions of that tool) can send from the batch script. As a backup (if not netcat can be found), a file is used.
  • Solves the issue of the state order inversion vs. when-is-the-native-id-available in the local executor by making the local executor template based (like the batch scheduler ones) and implementing staging as it is done in the batch scheduler executors. This has the disadvantage that it only delays finding a better solution for the native_id/order issue that will inevitably happen with synthetic-staging executors, but the plus is that the local executor is closer, for testing purposes, to the batch scheduler ones.
  • Adds some testing tools for managing temporary files/directories
  • Ensures that polling threads for batch scheduler executors are shut down when the executor is GC-ed

Copy link

codecov bot commented Feb 24, 2024

Codecov Report

Attention: Patch coverage is 89.25750% with 68 lines in your changes are missing coverage. Please review.

Project coverage is 76.66%. Comparing base (91ad420) to head (2190c87).
Report is 5 commits behind head on main.

Files Patch % Lines
src/psij/staging.py 82.60% 20 Missing ⚠️
src/psij/utils.py 91.04% 12 Missing ⚠️
tests/_test_tools.py 76.00% 12 Missing ⚠️
tests/conftest.py 31.25% 11 Missing ⚠️
...c/psij/executors/batch/batch_scheduler_executor.py 89.23% 7 Missing ⚠️
src/psij/executors/local.py 83.33% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #450      +/-   ##
==========================================
+ Coverage   74.69%   76.66%   +1.97%     
==========================================
  Files          94       96       +2     
  Lines        3912     4440     +528     
==========================================
+ Hits         2922     3404     +482     
- Misses        990     1036      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…eled

jobs and spam the pytest suite, since pytest doesn't capture the log after
the test is done.
special devices which bash will happily emulate if not found.
Since the addition of staging, errors can reasonably occur outside of
the launcher as well as inside the launcher and pre- and post- launch
scripts. So we treat the output of the whole script (including launcher)
as potential indicator of an error.
…s to

CWD (the CI runner runs things in /tmp).
…to avoid

clashes with other things that might be defined in the environment.
- use `StageOutFlags` to express when cleanup happens rather than a boolean
flag
- fixed equality operator in `JobSpec`
- added `__str__`, `__eq__` and `__hash__` methods for staging objects
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.

1 participant