-
Notifications
You must be signed in to change notification settings - Fork 537
Response and database improvements #3675
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
5196571
to
57d45b5
Compare
7ce68dc
to
953c1d7
Compare
ZenML CLI Performance Comparison (Threshold: 1.0s, Timeout: 60s, Slow: 5s)❌ Failed Commands on Current Branch (feature/response-sizes)
🚨 New Failures IntroducedThe following commands fail on your branch but worked on the target branch:
Performance Comparison
Summary
Environment Info
|
70f923e
to
9a5c558
Compare
91925d4
to
4970a35
Compare
77283a1
to
598b651
Compare
598b651
to
d1c959b
Compare
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.
I didn't do a thorough review, as many of these changes need deep model and DB schema knowledge. Looks alright otherwise.
from zenml.client import Client | ||
|
||
return Client().get_project(self.project_id) |
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.
Saving this in the object might help reduce the number of API/DB calls.
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.
This is used just once in our code, which is why I decided to not cache this. Do you still think it's necessary?
Describe changes
This PR improves the performance of the ZenML server by optimizing responses and DB queries.
Breaking changes
Model.get_pipeline_run(...)
as well asModelVersionResponse.get_pipeline_run(...)
have been removed. UseClient().list_pipeline_runs(name=...)
instead.Response changes
body
toresources
, which means they will not be included anymore if this model is included in a different response.Detailed list of response changes:
body.user
was moved toresources.user
and a new attributebody.user_id
was added.metadata.project
was removed. A newbody.project_id
attribute has been added as replacement.body.tags
got moved toresources.tags
body.latest_version_name
got moved toresources.latest_version_name
body.latest_version_id
got moved toresources.latest_version_id
body.tags
got moved toresources.tags
body.producer_pipeline_run_id
got moved toresources.producer_pipeline_run_id
metadata.producer_step_run_id
got moved toresources.producer_step_run_id
body.tags
got moved toresources.tags
body.latest_version_name
got moved toresources.latest_version_name
body.latest_version_id
got moved toresources.latest_version_id
body.model_artifact_ids
,body.data_artifact_ids
,body.deployment_artifact_ids
andbody.pipeline_run_ids
got removedbody.tags
got moved toresources.tags
body.latest_run_id
got moved toresources.latest_run_id
body.latest_run_status
got moved toresources.latest_run_status
resources.triggers
got removedmetadata.steps
andmetadata.step_substitutions
got removedbody.latest_run_id
got moved toresources.latest_run_id
body.latest_run_status
got moved toresources.latest_run_status
body.inputs
moved toresources.inputs
and changed type to allow multiple inputs for one namebody.outputs
moved toresources.outputs
body.substitutions
body.tagged_count
moved tometadata.tagged_count
Database changes:
Misc other changes:
GET /runs/<ID>/dag
has been added. This allows the frontend to fetch only relevant information for the DAG visualizer screen without fetching details about all steps in the pipeline run.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