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

Better files download and replication configurability #543

Open
aaaditij opened this issue Sep 11, 2024 · 2 comments
Open

Better files download and replication configurability #543

aaaditij opened this issue Sep 11, 2024 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@aaaditij
Copy link

aaaditij commented Sep 11, 2024

Problem

For the createjob api, one of the inputs to this API is a boolean flag called package_input_folder, which when set to true, packages the input folder (the folder containing the input notebook) and all nested files and subfolders within it during the job creation. This introduces the following problems:

  1. download_files api copies the entire input folder from staging area to the output folder. This is currently done so that notebook downloaded with other output files would have access to all the same files as original and so that running notebook as a whole or some cells could be replicated if they refer to files via local paths.
    This in essence is copying the entire input folder twice, once to the staging area
    and then to the output folder and can quickly lead to storage exhaustion if the input folder is large.

  2. The files in the staging area are never cleaned up again eating up storage space.

Proposed solution

  1. Add a boolean flag to download_files api to allow the user to specify if they only want the output files to be copied over to the output folder.
  2. Add a boolean flag to download_files api to delete all files belonging to an execution from the staging area after they have been copied over to the output folder.
@aaaditij aaaditij added the enhancement New feature or request label Sep 11, 2024
@andrii-i
Copy link
Collaborator

Hi @aaaditij. Thank you for creating this enhancement request. These options make sense to me as traitlet-configurable options. At the same time we are not working on any of them now and are not planning to as there are some high-priority deliverables in the pipeline.

@andrii-i andrii-i added this to the Future milestone Sep 19, 2024
@andrii-i
Copy link
Collaborator

andrii-i commented Sep 23, 2024

Implementation overview for "1.) Add a boolean flag to download_files api to allow the user to specify if they only want the output files to be copied over to the output folder." based on discussion with @aaaditij:

  1. Add optional side_effects : Optional[List[str]] = [] field to all Job-related data models https://github.com/jupyter-server/jupyter-scheduler/blob/main/jupyter_scheduler/models.py
  2. Use side_effects to store side effect files created during the job run instead of adding them to packaged_files by changing DefaultExecutionManager.add_side_effects_files accordingly
    def add_side_effects_files(self, staging_dir: str):
  3. Add config option (let’s call it download_output_files_only ) or API option by modify the download endpoint by changing FilesDownloadHandler to accept output files only option
    class FilesDownloadHandler(ExtensionHandlerMixin, APIHandler):
  4. Pass config option to JobFilesManager and Downloader as parameters, add them as arguments to both and any intermediary classes
    target=Downloader(
    output_formats=job.output_formats,
    output_filenames=output_filenames,
    staging_paths=staging_paths,
    output_dir=output_dir,
    redownload=redownload,
    include_staging_files=job.package_input_folder,
    ).download
  5. Change Downloader.generate_filepaths function to only return side effects and outputs and not packaged files if download_output_files_only is set
    def generate_filepaths(self):
    """A generator that produces filepaths"""
    output_formats = self.output_formats + ["input"]
    for output_format in output_formats:
    input_filepath = self.staging_paths[output_format]
    output_filepath = os.path.join(self.output_dir, self.output_filenames[output_format])
    if not os.path.exists(output_filepath) or self.redownload:
    yield input_filepath, output_filepath
    if self.include_staging_files:
    staging_dir = os.path.dirname(self.staging_paths["input"])
    for file_relative_path in self.output_filenames["files"]:
    input_filepath = os.path.join(staging_dir, file_relative_path)
    output_filepath = os.path.join(self.output_dir, file_relative_path)
    if not os.path.exists(output_filepath) or self.redownload:
    yield input_filepath, output_filepath
  6. Add a test for job files manager using the existing fixtures https://github.com/jupyter-server/jupyter-scheduler/blob/main/jupyter_scheduler/tests/test_job_files_manager.py#L137

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants