-
Notifications
You must be signed in to change notification settings - Fork 23
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
Archiving all-files scheduler #388
Archiving all-files scheduler #388
Conversation
for more information, see https://pre-commit.ci
202fe09
to
5ac1f74
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JasonWeill Thank you for working on this! Left some feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also a failing unit test blocking CI:
FAILED jupyter_scheduler/tests/test_job_files_manager.py::test_downloader_download[output_formats1-output_filenames1-staging_paths1-/home/runner/work/jupyter-scheduler/jupyter-scheduler/jupyter_scheduler/tests/test_files_output-False] - AssertionError: assert False
+ where False = <function exists at 0x7feb182f7420>('/home/runner/work/jupyter-scheduler/jupyter-scheduler/jupyter_scheduler/tests/test_files_output/helloworld-out.ipynb')
+ where <function exists at 0x7feb182f7420> = <module 'posixpath' (frozen)>.exists
+ where <module 'posixpath' (frozen)> = os.path
No need for an urgent fix here, since CI is failing on main
anyways until we merge #417. However, this definitely should be fixed after rebasing and before merging.
Co-authored-by: david qiu <[email protected]>
for more information, see https://pre-commit.ci
Simplifies archiving and unarchiving logic per @dlqqq. Seeking external feedback about having these new classes replace the old |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! 🎉
* Fix typo in comment * WIP: Adds new scheduler * writes individual files * WIP: Write zip file * WIP: Trying to get zip file to be written only on scheduled job runs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * WIP: Removes zip type, incremental work for archiving work dir * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Create tar.gz in staging subdir * Capture side effect files in staging dir * Extracts files * Add filter * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update jupyter_scheduler/job_files_manager.py Co-authored-by: david qiu <[email protected]> * Simplifies cleanup logic * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Updates docs, deletes old Archiving*, renames AllFilesArchiving --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: david qiu <[email protected]>
* Archiving all-files scheduler (#388) * Fix typo in comment * WIP: Adds new scheduler * writes individual files * WIP: Write zip file * WIP: Trying to get zip file to be written only on scheduled job runs * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * WIP: Removes zip type, incremental work for archiving work dir * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Create tar.gz in staging subdir * Capture side effect files in staging dir * Extracts files * Add filter * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update jupyter_scheduler/job_files_manager.py Co-authored-by: david qiu <[email protected]> * Simplifies cleanup logic * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Updates docs, deletes old Archiving*, renames AllFilesArchiving --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: david qiu <[email protected]> * Avoids option compatible only with Python 3.11 * Fix JFM tests (#424) * fix JFM tests * pre-commit * add minor comment --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: david qiu <[email protected]>
WIP fix for #349.
Creates an
AllFilesArchivingExecutionManager
andAllFilesArchivingScheduler
class, with the intention of archiving all output files and side effects on job runs.You can use these classes by running:
After the user chooses to download output files from a job, the side effect files are unpacked to wherever the first output file is located.
In the screen shot below, after downloading the output of a job that ran
writefile.ipynb
, which produces the side effect filepun.txt
, the side effect file is in thejobs
subdirectory.