Feature/revamp model endpoints #2035
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
internal
|
@avishniakov please let me know if I am missing anything obvious - I have opted to keep the behaviour for get and list methods consistent, but everything else now relies on the new more atomic interface |
avishniakov
approved these changes
Nov 9, 2023
avishniakov
suggested changes
Nov 9, 2023
Co-authored-by: Andrei Vishniakov <[email protected]>
…zenml into feature/revamp-model-endpoints
|
@avishniakov do you have any manual testing flow that you used, that I can do to ensure no functionality is lost? |
I think, you can run the E2E template on a remote (docker) setup - should be good enough. Otherwise, it is pretty well covered with tests already. |
…' into feature/revamp-model-endpoints
AlexejPenner
changed the base branch from
develop
to
feature/OSS-2609-OSS-2575-model-config-is-model-version
…' into feature/revamp-model-endpoints
AlexejPenner
commented
Nov 15, 2023
Comment on lines
+240
to
+242
| mv._id = self.id | ||
|
|
||
| return mv |
There was a problem hiding this comment.
@avishniakov I discovered this value was never set, i implemented a temporary fix but would leave it up to you to decide where to handle this
|
E2E template updates in |
…zenml into feature/revamp-model-endpoints
…' into feature/revamp-model-endpoints
|
I will bring it to feature/OSS-2609-OSS-2575-model-config-is-model-version and rebase with hydration there. It will speed us up. |
avishniakov
merged commit 7f1642c
into
feature/OSS-2609-OSS-2575-model-config-is-model-version
5 of 17 checks passed
avishniakov
added a commit
that referenced
this pull request
Nov 17, 2023
* rename file * rename file * rename file * big rebranding * big rebranding * deprecate method * redesign context to return ModelVersion * add `linked_artifact_ids` property * add `__eq__` * fix typo * add number and name props * rename artifact types * prevent reserved mv names creation * typo * more logging * prevent reserved mv names creation * handle reserved names properly * rework client calls * remove `version_description` * remove `version_name` * `delete_new_version_on_failure` > `with_recovery` * fix alembic false alarms * output alembic branches * resolve branching * update test signature * Temporarily fix quickstart until the certificate is renewed * add existing_type * enforce tags update * update template tag * after merge mess * resolve branching * finish renaming * revert `log_artifact_metadata` * tagging bugfix * rework client methods * sunset recovery and `running` * nits from review * rename files * rename vars * remove `ReservedNameError` * resolve branching * Auto-update of E2E template * fixing bugs caught by tests * fix branching * properly refresh template * overcome pagination issue in all_model_versions * rename misleading method * remove `all_model_versions` * update `list_models` * improve docstring * rephrase docstring * nits * Auto-update of E2E template * stick to unified list interface * Feature/revamp model endpoints (#2035) * Moved model versions to their own root route * Fix the delete endpoint * Fixed smaller mistakes * Fixed for Client as well * Fixed lint errors * More linting * More linting * More tiny fixes * Update src/zenml/zen_server/routers/model_versions_endpoints.py Co-authored-by: Andrei Vishniakov <[email protected]> * More linting * Fixed tests and solved conflicts * Fixed linting * Fixed more tests * Further refactoring * Added raises section * Fix one failing test * Take "latest" stage into account * Reformatted * Standardize use of list response * Rewrote some tests * Add clien tests * Fixed spelling * Tested to work with e2e pipeline * Ugly fixes to get response models in the CLI * Auto-update of E2E template * Access ModelVersionResponseModels in Client again * Another small fix * Linted --------- Co-authored-by: Andrei Vishniakov <[email protected]> Co-authored-by: GitHub Actions <[email protected]> * resolve mess after merge * after merge clean-up * update outdated tests * Auto-update of E2E template * deep update of template * Revert "deep update of template" This reverts commit b518dcc. --------- Co-authored-by: Stefan Nica <[email protected]> Co-authored-by: GitHub Actions <[email protected]> Co-authored-by: Alexej Penner <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe changes
I made model_versions their own root route - for the purpose of preserving some of the implicit functionality of model versions I left two endpoints on the /models route to allow users to fetch model versions based on latest, stage or name
We need to have all the model_version endpoints with a root level to allow for all sort of operations through the dashboard.
Pre-requisites
Please ensure you have done the following:
developand the open PR is targetingdevelop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes