Create tags table #2036

avishniakov · 2023-11-09T15:04:20Z

Describe changes

I implemented Tags and TagsResources tables as follows:

The connection between Models (as a first entity) and linking table TagResource is soft and controlled by SQLModel, shown only for demo purposes - it is not a real FK on the DB level.

Tags are equipped with the full scope of CLI commands:

zenml tag --help
Usage: zenml tag [OPTIONS] COMMAND [ARGS]...

  Interact with tags.

Options:
  --help  Show this message and exit.

Commands:
  delete    Delete an existing tag.
  list      List tags with filter.
  register  Register a new tag.
  update    Update an existing tag.

How does the connection between entities and tags work?
TagResources

    resource_id: UUID
    resource_type: int
    model: List["ModelSchema"] = Relationship(
        back_populates="tags",
        sa_relationship_kwargs=dict(
            primaryjoin=f"and_(TagResourceSchema.resource_type=={TaggableResourceTypes.MODEL.value}, foreign(TagResourceSchema.resource_id)==ModelSchema.id)",
        ),
    )

Models (same for any other entity)

tags: List["TagResourceSchema"] = Relationship(
        back_populates="model",
        sa_relationship_kwargs=dict(
            primaryjoin=f"and_(TagResourceSchema.resource_type=={TaggableResourceTypes.MODEL.value}, foreign(TagResourceSchema.resource_id)==ModelSchema.id)",
            cascade="delete",
        ),
    )

What are the needed changes for adding other Taggable entities?

Extend zenml.enums.TaggableResourceTypes
Add relationship between tables as shown above
On update handle tags separately inside update in SQL store:

            if model_update.add_tags:
                self._attach_tags_to_resource(
                    tag_names=model_update.add_tags,
                    resource_id=existing_model.id,
                    resource_type=TaggableResourceTypes.MODEL,
                )
            model_update.add_tags = None
            if model_update.remove_tags:
                self._detach_tags_from_resource(
                    tag_names=model_update.remove_tags,
                    resource_id=existing_model.id,
                    resource_type=TaggableResourceTypes.MODEL,
                )
            model_update.remove_tags = None

            existing_model.update(model_update=model_update)

Update object creation codes:

if model.tags:
    attach_tags_to_resource(
        tag_names=model.tags,
        resource_id=model_schema.id,
        resource_type=TaggableResourceTypes.MODEL,
        sql_store=self,
    )
session.commit()

@Cahllagerfeld , this PR changes the Update model for Models by removing tags and introducing add_tags and remove_tags

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)

AlexejPenner

Just a general question, how implicit should this whole tagging be. The way I alway imagined it was when I create a resource with a list of tags (strings) the whole db entity creation happens completely abstracted away from me as a user. Even at the API layer I would imagine that we use updates on the resource to set/remove tags. What do you think about this in general.

src/zenml/enums.py

AlexejPenner · 2023-11-09T15:45:24Z

src/zenml/utils/tag_utils.py

+            pass
+
+
+def delete_links(


No idea where to write this - but do we have any way of implementing cascading delete? Like if I delete resource 1 - will all entries for tags on resource 1 be deleted

It will be cascaded - this is need for removal of tags via update, I'll rename it for clarity

This folk is cascading (set in Models and other taggable entities going forward)

tags: List["TagResourceSchema"] = Relationship( back_populates="model", sa_relationship_kwargs=dict( primaryjoin=f"and_(TagResourceSchema.resource_type=={TaggableResourceTypes.MODEL.value}, foreign(TagResourceSchema.resource_id)==ModelSchema.id)", cascade="delete", ), )

AlexejPenner · 2023-11-09T15:51:48Z

src/zenml/cli/model.py

@@ -267,7 +267,7 @@ def update_model(
            trade_offs=tradeoffs,
            ethics=ethical,
            limitations=limitations,
-            tags=tag,
+            add_tags=tag,


I would expect this to take a list of strings not just one string

It is a list, just the limitation of click - the name of arg must match the name of arg in the function and the user pass it as -t a -t b ...

avishniakov · 2023-11-09T16:18:51Z

Just a general question, how implicit should this whole tagging be. The way I alway imagined it was when I create a resource with a list of tags (strings) the whole db entity creation happens completely abstracted away from me as a user. Even at the API layer I would imagine that we use updates on the resource to set/remove tags. What do you think about this in general.

It is exactly like you described - the user passes in a list of strings, and creation/retrieval happens under the hood. Next with Update models one can add or remove tags

fa9r

Awesome, love the overall design, very clean how tags and resources are connected now, well done!!

Overall, the entire PR looks great and very polished already. High-level the only change I'd request is to handle the link creation inside the update_{resource} ZenStore methods and get rid of the TagResource endpoints.

Also, I noticed the RestZenStore methods are still missing, but I'm sure integration tests would reveal that sooner or later anyways.

fa9r · 2023-11-10T08:37:28Z

src/zenml/models/model_models.py

+    tags: List[TagResponseModel] = Field(
+        title="Tags associated with the model",
+    )
+    tagged: List[str]


tagged seems redundant, this is just tag.name for tag in tags right?

This is for CLI only, yes it is a bit redundant

In that case I'd suggest to delete that arg and simply call tag.name for tag in tags wherever you need it

src/zenml/models/tag_models.py

src/zenml/enums.py

src/zenml/utils/tag_utils.py

src/zenml/zen_server/routers/tags_endpoints.py

src/zenml/zen_stores/schemas/__init__.py

src/zenml/zen_stores/zen_store_interface.py

src/zenml/zen_stores/schemas/model_schemas.py

fa9r · 2023-11-10T09:34:08Z

Just a general question, how implicit should this whole tagging be. The way I alway imagined it was when I create a resource with a list of tags (strings) the whole db entity creation happens completely abstracted away from me as a user. Even at the API layer I would imagine that we use updates on the resource to set/remove tags. What do you think about this in general.

Yes, that's exactly what I meant as well with links being created in update methods instead 👍

fa9r

Beautiful 🙌

fa9r · 2023-11-10T12:39:41Z

src/zenml/zen_stores/sql_zen_store.py

@@ -6560,14 +6563,17 @@ def create_tag_resource(
    def delete_tag_resource(


Is this method and the other still used now that the interface no longer exists?

Yeah, called inside helpers.

github-actions · 2023-11-10T17:34:31Z

E2E template updates in examples/e2e have been pushed.

stefannica

Beautifully crafted PR ! I only have a couple of suggestions, but nothing that would keep this from being merged.

src/zenml/zen_stores/schemas/tag_schemas.py

src/zenml/utils/tag_utils.py

avishniakov added 4 commits November 9, 2023 09:09

working option 2

c9729d9

working option 2

caecabc

working option 2

27624c6

fix mysql

15df83c

avishniakov changed the base branch from main to develop November 9, 2023 15:04

github-actions bot added internal To filter out internal PRs and issues enhancement New feature or request labels Nov 9, 2023

Merge branch 'develop' into feature/OSS-2610-create-tags-table

a813455

avishniakov requested review from stefannica, AlexejPenner and fa9r November 9, 2023 15:05

AlexejPenner reviewed Nov 9, 2023

View reviewed changes

fix bug + add tests

331e1a1

rename

7ab1a22

avishniakov requested a review from AlexejPenner November 9, 2023 16:20

avishniakov added 4 commits November 10, 2023 07:54

Merge branch 'develop' into feature/OSS-2610-create-tags-table

200776e

add alembic branches output on divergence

a265b64

add client functions

4ff8b26

strenums for types/colors

4438a5f

fa9r suggested changes Nov 10, 2023

View reviewed changes

avishniakov added 6 commits November 10, 2023 10:39

add tags cli

9f7daf6

add tags cli

e9b539a

try bypass alembic branching

da14b2b

remove tag<>resource endpoints

fb26bd4

rely on sql for tag links

df82b6e

Merge branch 'develop' into feature/OSS-2610-create-tags-table

1dd232b

avishniakov requested a review from fa9r November 10, 2023 11:54

fa9r approved these changes Nov 10, 2023

View reviewed changes

avishniakov and others added 9 commits November 10, 2023 15:19

fix migration bug with uuids

17bced5

remove tagged

3bb6f82

calm down branching check on zenml import

a8f508e

update signature in tests

3206946

Merge branch 'develop' into feature/OSS-2610-create-tags-table

994b59a

update signature in tests

ee57e87

Merge branch 'develop' into feature/OSS-2610-create-tags-table

7033f23

resolve branching

4b51589

Auto-update of E2E template

81a0390

stefannica approved these changes Nov 13, 2023

View reviewed changes

src/zenml/zen_stores/schemas/tag_schemas.py Show resolved Hide resolved

src/zenml/utils/tag_utils.py Show resolved Hide resolved

avishniakov and others added 11 commits November 13, 2023 09:19

move tagging code to sql store

de13f1e

resolve branching

4adcd0b

Merge branch 'develop' into feature/OSS-2610-create-tags-table

dce6439

resolve alembic

3f79f50

stabilize test case

08649c8

better cleanups in tests

005adcb

workaround fix for quickstart

d6e76aa

revert hard cleanup

5324f2b

explicit asserts in cli

1355e7c

revert workaround fix for quickstart

f388ae0

Temporarily fix quickstart until the certificate is renewed

ed33c9f

avishniakov merged commit d847b31 into develop Nov 14, 2023
29 checks passed

avishniakov deleted the feature/OSS-2610-create-tags-table branch November 14, 2023 05:18

fa9r mentioned this pull request Nov 29, 2023

Artifact Version Table + Artifact Tagging #2081

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create tags table #2036

Create tags table #2036

avishniakov commented Nov 9, 2023 •

edited

Loading

AlexejPenner left a comment

AlexejPenner Nov 9, 2023

avishniakov Nov 9, 2023

avishniakov Nov 9, 2023

AlexejPenner Nov 9, 2023

avishniakov Nov 9, 2023

avishniakov commented Nov 9, 2023

fa9r left a comment

fa9r Nov 10, 2023

avishniakov Nov 10, 2023

fa9r Nov 10, 2023

avishniakov Nov 10, 2023

fa9r commented Nov 10, 2023

fa9r left a comment

fa9r Nov 10, 2023

avishniakov Nov 10, 2023

github-actions bot commented Nov 10, 2023

stefannica left a comment

		@@ -6560,14 +6563,17 @@ def create_tag_resource(
		def delete_tag_resource(

Create tags table #2036

Create tags table #2036

Conversation

avishniakov commented Nov 9, 2023 • edited Loading

Describe changes

Pre-requisites

Types of changes

AlexejPenner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avishniakov commented Nov 9, 2023

fa9r left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fa9r commented Nov 10, 2023

fa9r left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 10, 2023

stefannica left a comment

Choose a reason for hiding this comment

avishniakov commented Nov 9, 2023 •

edited

Loading