Model Versions are taggable

[avishniakov]

Describe changes

I implemented tags support for Model Versions in Client, CLI, and endpoints.
New Model Version will also get tags from ModelVersion on every run, if Model Version or Model exists - tag has to be updated by CLI/ Client/Frontend instead.

Pre-requisites

Please ensure you have done the following:

• I have read the CONTRIBUTING.md document.
• If my change requires a change to docs, I have updated the documentation accordingly.
• If I have added an integration, I have updated the integrations table and the corresponding website section.
• I have added tests to cover my changes.
• I have based my new branch on develop and the open PR is targeting develop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Other (add details above)

[review-notebook-app]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[htahir1]

@avishniakov I havnt looked at the code but from your description I have a question:

if Model Version or Model exists - tag has to be updated by CLI/ Client/Frontend instead.

Dont you think if i explcitly pass tags and specify the model and model version in the @pipeline decorator these should be overwritten?

[avishniakov]

@avishniakov I havnt looked at the code but from your description I have a question:

if Model Version or Model exists - tag has to be updated by CLI/ Client/Frontend instead.

Dont you think if i explcitly pass tags and specify the model and model version in the @pipeline decorator these should be overwritten?

I, personally, don't think so. Someone (maybe not you) created this model version and you only use it later on, why should you be in the power to override what was defined by the model version creator?
I was also thinking about adding new tags, but it will become messy soon if people misuse them.
Shall I raise this in a feedback and engage Zuri, maybe?

[htahir1]

@avishniakov I think its a good idea to ask Zuri, but it might also be a permission scope thing right? Like maybe you dont have the ability to edit the model so then your pipeline run will fail?

What is the current behavior? Does the pipeline fail or just ignore/give a warning? If its the latter, then im happy to keep that behavior

[avishniakov]

@avishniakov I think its a good idea to ask Zuri, but it might also be a permission scope thing right? Like maybe you dont have the ability to edit the model so then your pipeline run will fail?

What is the current behavior? Does the pipeline fail or just ignore/give a warning? If its the latter, then im happy to keep that behavior

Now it just ignoring tags - they are used only if new ModelVersion/Model is created

[htahir1]

@avishniakov ok then lets leave that behavior until someone else objects

[strickvl]

LGTM aside from two things:

1. you've update the schemas, so doesn't this require a DB migration?
2. could you update the src/zenml/cli/__init__.py to include instructions on adding / using these tags in the appropriate section?

[avishniakov]

LGTM aside from two things:

1. you've update the schemas, so doesn't this require a DB migration?
2. could you update the src/zenml/cli/__init__.py to include instructions on adding / using these tags in the appropriate section?

1. It is all "virtual" references - alembic produces nothing on this change
2. I added a section to CLI docs since it was never there.

[strickvl]

Right got it. It's just creating the new fields from things it already has rather than setting up new places to store things. Nice one!

[bcdurak]

Overall looks very clean and helpful :) Great changes. Outside of a few minor comments, LGTM!

[bcdurak]

Also a general question here: I started seeing this pattern with the other taggable entities as well. In the earlier iterations, we have used the update method of a Schema to apply such changes. Lately, I see that the update is happening one layer up on the SqlZenStore level instead of the Schema level. Is there a reason why we see this switch when it comes to models and artifacts?

[avishniakov]

Not sure that there is a real explanation behind that. Can you share some Schemas having such "extended" updates? For instance in run_metadata we use an extra method to create those, so it is not really closely coupled with the object itself (e.g. create_run_metadata is called as one of the steps in save_artifact utility).
It might turn out that having those very related entities coupled in one update method is a bit better choice, IMO. But it is definitely not a battle I want to fight in, so if you share examples and reject my explanations - I will rework 👍🏼

[bcdurak]

I think in their first iterations, most schemas had an update function which was relatively more extended. However, as I went through the current implementation, I see that this is not the case anymore. I don't think we need to rework anything in the scope of this PR though, ideally, this would be a separate cleaning-up ticket in the future.

[bcdurak]

If we need examples in any case:

• Code repository schemas: The entire update on the schema happens within the update method not in the SQLZenStore.
• Stack component schemas: The update on the configuration and labels are happening inside the update function whereas the update on the connectors is happening on the SQLZenStore.,

This is the inconsistency that I find suboptimal. IMO, the update should either happen fully on the SQLZenStore or fully on the update method of a schema. Otherwise, the code becomes very hard to track and read.

[github-actions]

E2E template updates in examples/e2e have been pushed.