-
Notifications
You must be signed in to change notification settings - Fork 537
Add endpoint to download artifact data #3507
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
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
@htahir1 @stefannica Should we limit this to artifacts of a certain size? This will download all the artifact data on the server pod, which will cause issues for larger artifacts. |
@schustmi Great idea! Lets make that even a configurable env variable for the server. WDYT? And have the default be 2 GB even? I think up to that should be fine |
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.
love this with the jwt stuff too! im not sure if it needs a docs page though? with ENV_ZENML_SERVER_FILE_DOWNLOAD_SIZE_LIMIT documented?
src/zenml/artifacts/utils.py
Outdated
max_download_size = handle_int_env_var( | ||
ENV_ZENML_SERVER_FILE_DOWNLOAD_SIZE_LIMIT, | ||
DEFAULT_ZENML_SERVER_FILE_DOWNLOAD_SIZE_LIMIT, | ||
) |
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.
You should use a ServerConfiguration setting for this.
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.
Done
ZenML CLI Performance Comparison (Threshold: 1.0s, Timeout: 60s, Slow: 5s)❌ Failed Commands on Current Branch (feature/artifact-data-download)
🚨 New Failures IntroducedThe following commands fail on your branch but worked on the target branch:
Performance Comparison
Summary
Environment Info
|
Describe changes
This PR adds an endpoint to download artifact data.
Pre-requisites
Please ensure you have done the following:
develop
and the open PR is targetingdevelop
. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes