sadpandajoe commented on code in PR #42919:
URL: https://github.com/apache/superset/pull/42919#discussion_r3820906177
##########
superset/commands/importers/v1/utils.py:
##########
@@ -318,28 +318,29 @@ def import_tag(
for tag_name in target_tag_names:
try:
- tag = existing_tags.get(tag_name)
-
- # If tag does not exist, create it
- if tag is None:
- description = tag_descriptions.get(tag_name, None)
- tag = Tag(name=tag_name, description=description,
type="custom")
- db_session.add(tag)
- existing_tags[tag_name] = tag # Update the existing_tags
dictionary
-
- # Ensure the association with the object
- tagged_object = (
- db_session.query(TaggedObject)
- .filter_by(object_id=object_id, object_type=object_type,
tag_id=tag.id)
- .first()
- )
- if not tagged_object:
- new_tagged_object = TaggedObject(
- tag_id=tag.id, object_id=object_id, object_type=object_type
+ with db_session.begin_nested():
+ tag = existing_tags.get(tag_name)
+
+ # If tag does not exist, create it
+ if tag is None:
+ description = tag_descriptions.get(tag_name, None)
+ tag = Tag(name=tag_name, description=description,
type="custom")
+ db_session.add(tag)
+ existing_tags[tag_name] = tag # Update the existing_tags
dict
Review Comment:
Agreed—the savepoint rolls back the newly inserted tag but leaves it cached,
so a concurrent create makes this import skip the requested association. Could
this re-query the tag after the uniqueness conflict before continuing?
##########
superset/commands/importers/v1/utils.py:
##########
@@ -318,28 +318,29 @@ def import_tag(
for tag_name in target_tag_names:
try:
- tag = existing_tags.get(tag_name)
-
- # If tag does not exist, create it
- if tag is None:
- description = tag_descriptions.get(tag_name, None)
- tag = Tag(name=tag_name, description=description,
type="custom")
- db_session.add(tag)
- existing_tags[tag_name] = tag # Update the existing_tags
dictionary
-
- # Ensure the association with the object
- tagged_object = (
- db_session.query(TaggedObject)
- .filter_by(object_id=object_id, object_type=object_type,
tag_id=tag.id)
- .first()
- )
- if not tagged_object:
- new_tagged_object = TaggedObject(
- tag_id=tag.id, object_id=object_id, object_type=object_type
+ with db_session.begin_nested():
+ tag = existing_tags.get(tag_name)
+
+ # If tag does not exist, create it
+ if tag is None:
+ description = tag_descriptions.get(tag_name, None)
+ tag = Tag(name=tag_name, description=description,
type="custom")
+ db_session.add(tag)
+ existing_tags[tag_name] = tag # Update the existing_tags
dict
+
+ # Ensure the association with the object
+ tagged_object = (
+ db_session.query(TaggedObject)
+ .filter_by(object_id=object_id, object_type=object_type,
tag_id=tag.id)
+ .first()
)
- db_session.add(new_tagged_object)
+ if not tagged_object:
+ new_tagged_object = TaggedObject(
+ tag_id=tag.id, object_id=object_id,
object_type=object_type
+ )
+ db_session.add(new_tagged_object)
- new_tag_ids.append(tag.id)
+ new_tag_ids.append(tag.id)
Review Comment:
Agreed—an error while releasing the savepoint rolls back the association,
but its tag ID has already been recorded as successful. Could this append only
after the nested block exits?
--
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]
