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

Issue 843 | Files: download files saved during Program execution #896

Merged
merged 12 commits into from
Sep 1, 2023

Conversation

IceKhan13
Copy link
Member

@IceKhan13 IceKhan13 commented Aug 21, 2023

Summary

Add feature to download save files to /data folder within programs.

Example:

serverless.files()
serverless.download("my_awesome.tar")

Limitations:

  • only tar files are supported
  • tar file should be saved in /data directory during your program execution to be visible by .files() method call
  • only /data directory is supported, /data/other_folder will not be visible

Todos:

  • client methods
  • gateway files api
  • gateway file api tests
  • doc/tutorial

Details and comments

Closes #843

@IceKhan13 IceKhan13 added the enhancement New feature or request label Aug 21, 2023
@IceKhan13 IceKhan13 added this to the 0.4 milestone Aug 21, 2023
@IceKhan13 IceKhan13 changed the title Issue 843 | Client: files Issue 843 | Files: download files saved in Program executions Aug 21, 2023
@IceKhan13 IceKhan13 changed the title Issue 843 | Files: download files saved in Program executions Issue 843 | Files: download files saved during Program execution Aug 24, 2023
@IceKhan13 IceKhan13 marked this pull request as ready for review August 31, 2023 17:41
client/quantum_serverless/core/files.py Show resolved Hide resolved
gateway/api/v1/views.py Show resolved Hide resolved
gateway/api/views.py Outdated Show resolved Hide resolved
gateway/api/views.py Show resolved Hide resolved
gateway/tests/api/test_files.py Show resolved Hide resolved
progress_bar = tqdm(
total=total_size_in_bytes, unit="iB", unit_scale=True
)
file_name = f"downloaded_{str(uuid.uuid4())[:8]}_{file}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this download into an unique file name? Avoiding override or appending?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, to avoid overriding and we also return file name too, if user want to do something with it programmatically

IceKhan13 and others added 2 commits August 31, 2023 17:46
Co-authored-by: Paul Schweigert <paul@paulschweigert.com>
with open("./my_file.txt", "w") as f:
f.write("Hello!")

with tarfile.open("/data/my_file.tar", "w:gz") as tar:
Copy link
Collaborator

Choose a reason for hiding this comment

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

The file in the data directory is not deleted after this execution. The next execution of the same program (or generating the same name file) override the file. It it fails to generate the file, the old file may be down loaded. It is user responsibility to generate an unique file name so that it would not be override or confused?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, it is up to user to handle their files and names of them inside the program.

Copy link
Collaborator

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

LGTM
(since @akihikokuroda also had comments, will let him give final approval)

"""
return self._selected_provider.files()

def download(self, file: str, download_location: str = "./"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is adding the delete after download option a bod idea?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think if user downloads the file, he probably need it 😄
Probably we do not need this flag

Copy link
Collaborator

Choose a reason for hiding this comment

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

How he can delete it if he is done with it?

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, you mean delete from server. For now bucket policy will delete it or user can remove it in a job

Copy link
Member Author

Choose a reason for hiding this comment

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

if we want to delete it it's probably worth createing follow up issue request to add /delete endpoint for files

Copy link
Member Author

Choose a reason for hiding this comment

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

and probably /upload would be great to add too

Copy link
Member Author

Choose a reason for hiding this comment

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

but so far we only had download request :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@akihikokuroda akihikokuroda left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

@IceKhan13 IceKhan13 merged commit eb79a4b into main Sep 1, 2023
@IceKhan13 IceKhan13 deleted the issue-843/files-download branch September 1, 2023 16:24
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

Successfully merging this pull request may close these issues.

Client: allow users download files saved in /data directory
3 participants