Vitor-Avila commented on code in PR #35541:
URL: https://github.com/apache/superset/pull/35541#discussion_r2409190196
##########
superset/tags/models.py:
##########
@@ -130,17 +130,24 @@ def __str__(self) -> str:
return f"<TaggedObject: {self.object_type}:{self.object_id}
TAG:{self.tag_id}>"
-def get_tag(
+def get_or_create_tag(
name: str,
session: orm.Session, # pylint: disable=disallowed-name
type_: TagType,
) -> Tag:
+ from sqlalchemy.exc import IntegrityError
+
tag_name = name.strip()
tag = session.query(Tag).filter_by(name=tag_name, type=type_).one_or_none()
if tag is None:
tag = Tag(name=escape(tag_name), type=type_)
session.add(tag)
- session.commit()
+ try:
+ session.commit()
+ except IntegrityError:
+ # Another transaction created the tag concurrently, fetch it
+ session.rollback()
+ tag = session.query(Tag).filter_by(name=tag_name, type=type_).one()
Review Comment:
I see we were already committing here before, but given this might be called
by a command (that has the `@transaction` decorator), should we have a `commit`
bool to only commit here if it's not called by a command? That would help
avoiding to create a new tag if the asset update/creation fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
