Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: harmonize python enum naming conventions #26948

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions superset/commands/chart/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def run(self) -> Model:

# Update tags
if (tags := self._properties.pop("tags", None)) is not None:
update_tags(ObjectType.chart, self._model.id, self._model.tags, tags)
update_tags(ObjectType.CHART, self._model.id, self._model.tags, tags)

if self._properties.get("query_context_generation") is None:
self._properties["last_saved_at"] = datetime.now()
Expand All @@ -79,6 +79,7 @@ def validate(self) -> None:

# Validate if datasource_id is provided datasource_type is required
datasource_id = self._properties.get("datasource_id")
datasource_type: str | None = None
if datasource_id is not None:
datasource_type = self._properties.get("datasource_type", "")
if not datasource_type:
Expand Down Expand Up @@ -106,12 +107,12 @@ def validate(self) -> None:

# validate tags
try:
validate_tags(ObjectType.chart, self._model.tags, tag_ids)
validate_tags(ObjectType.CHART, self._model.tags, tag_ids)
except ValidationError as ex:
exceptions.append(ex)

# Validate/Populate datasource
if datasource_id is not None:
if datasource_id is not None and datasource_type is not None:
Comment on lines -114 to +117
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bycatch, a more recent linter would have likely started complaining about this sooner or later.

try:
datasource = get_datasource_by_id(datasource_id, datasource_type)
self._properties["datasource_name"] = datasource.name
Expand Down
4 changes: 2 additions & 2 deletions superset/commands/dashboard/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def run(self) -> Model:

# Update tags
if (tags := self._properties.pop("tags", None)) is not None:
update_tags(ObjectType.dashboard, self._model.id, self._model.tags, tags)
update_tags(ObjectType.DASHBOARD, self._model.id, self._model.tags, tags)

dashboard = DashboardDAO.update(self._model, self._properties)
if self._properties.get("json_metadata"):
Expand Down Expand Up @@ -103,7 +103,7 @@ def validate(self) -> None:

# validate tags
try:
validate_tags(ObjectType.dashboard, self._model.tags, tag_ids)
validate_tags(ObjectType.DASHBOARD, self._model.tags, tag_ids)
except ValidationError as ex:
exceptions.append(ex)

Expand Down
2 changes: 1 addition & 1 deletion superset/commands/tag/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def run(self) -> tuple[set[tuple[str, int]], set[tuple[str, int]]]:
self.validate()

tag_name = self._properties["name"]
tag = TagDAO.get_by_name(tag_name.strip(), TagType.custom)
tag = TagDAO.get_by_name(tag_name.strip(), TagType.CUSTOM)
TagDAO.create_tag_relationship(
objects_to_tag=self._properties.get("objects_to_tag", []),
tag=tag,
Expand Down
6 changes: 3 additions & 3 deletions superset/commands/tag/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ def to_object_type(object_type: Union[ObjectType, int, str]) -> Optional[ObjectT
def to_object_model(
object_type: ObjectType, object_id: int
) -> Optional[Union[Dashboard, SavedQuery, Slice]]:
if ObjectType.dashboard == object_type:
if ObjectType.DASHBOARD == object_type:
return DashboardDAO.find_by_id(object_id)
if ObjectType.query == object_type:
if ObjectType.QUERY == object_type:
return SavedQueryDAO.find_by_id(object_id)
if ObjectType.chart == object_type:
if ObjectType.CHART == object_type:
return ChartDAO.find_by_id(object_id)
return None
8 changes: 4 additions & 4 deletions superset/commands/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def validate_tags(
return

# No changes in the list
current_custom_tags = [tag.id for tag in current_tags if tag.type == TagType.custom]
current_custom_tags = [tag.id for tag in current_tags if tag.type == TagType.CUSTOM]
if Counter(current_custom_tags) == Counter(new_tag_ids):
return

Expand All @@ -140,7 +140,7 @@ def validate_tags(
or security_manager.can_access("can_tag", object_type.name.capitalize())
):
validation_error = (
f"You do not have permission to manage tags on {object_type.name}s"
f"You do not have permission to manage tags on {object_type.name.lower()}s"
)
raise TagForbiddenError(validation_error)

Expand Down Expand Up @@ -169,9 +169,9 @@ def update_tags(
:param new_tag_ids: list of tags specified in the update payload
"""

current_custom_tags = [tag for tag in current_tags if tag.type == TagType.custom]
current_custom_tags = [tag for tag in current_tags if tag.type == TagType.CUSTOM]
current_custom_tag_ids = [
tag.id for tag in current_tags if tag.type == TagType.custom
tag.id for tag in current_tags if tag.type == TagType.CUSTOM
]

tags_to_delete = [tag for tag in current_custom_tags if tag.id not in new_tag_ids]
Expand Down
22 changes: 11 additions & 11 deletions superset/common/tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def add_types_to_charts(
[
tag.c.id.label("tag_id"),
slices.c.id.label("object_id"),
literal(ObjectType.chart.name).label("object_type"),
literal(ObjectType.CHART.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -67,7 +67,7 @@ def add_types_to_dashboards(
[
tag.c.id.label("tag_id"),
dashboard_table.c.id.label("object_id"),
literal(ObjectType.dashboard.name).label("object_type"),
literal(ObjectType.DASHBOARD.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -99,7 +99,7 @@ def add_types_to_saved_queries(
[
tag.c.id.label("tag_id"),
saved_query.c.id.label("object_id"),
literal(ObjectType.query.name).label("object_type"),
literal(ObjectType.QUERY.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -131,7 +131,7 @@ def add_types_to_datasets(
[
tag.c.id.label("tag_id"),
tables.c.id.label("object_id"),
literal(ObjectType.dataset.name).label("object_type"),
literal(ObjectType.DATASET.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -223,7 +223,7 @@ def add_types(metadata: MetaData) -> None:
insert = tag.insert()
for type_ in ObjectType.__members__:
with contextlib.suppress(IntegrityError): # already exists
db.session.execute(insert, name=f"type:{type_}", type=TagType.type)
db.session.execute(insert, name=f"type:{type_}", type=TagType.TYPE)

add_types_to_charts(metadata, tag, tagged_object, columns)
add_types_to_dashboards(metadata, tag, tagged_object, columns)
Expand All @@ -241,7 +241,7 @@ def add_owners_to_charts(
[
tag.c.id.label("tag_id"),
slices.c.id.label("object_id"),
literal(ObjectType.chart.name).label("object_type"),
literal(ObjectType.CHART.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -277,7 +277,7 @@ def add_owners_to_dashboards(
[
tag.c.id.label("tag_id"),
dashboard_table.c.id.label("object_id"),
literal(ObjectType.dashboard.name).label("object_type"),
literal(ObjectType.DASHBOARD.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -313,7 +313,7 @@ def add_owners_to_saved_queries(
[
tag.c.id.label("tag_id"),
saved_query.c.id.label("object_id"),
literal(ObjectType.query.name).label("object_type"),
literal(ObjectType.QUERY.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -349,7 +349,7 @@ def add_owners_to_datasets(
[
tag.c.id.label("tag_id"),
tables.c.id.label("object_id"),
literal(ObjectType.dataset.name).label("object_type"),
literal(ObjectType.DATASET.name).label("object_type"),
]
)
.select_from(
Expand Down Expand Up @@ -444,7 +444,7 @@ def add_owners(metadata: MetaData) -> None:
insert = tag.insert()
for (id_,) in db.session.execute(ids):
with contextlib.suppress(IntegrityError): # already exists
db.session.execute(insert, name=f"owner:{id_}", type=TagType.owner)
db.session.execute(insert, name=f"owner:{id_}", type=TagType.OWNER)
add_owners_to_charts(metadata, tag, tagged_object, columns)
add_owners_to_dashboards(metadata, tag, tagged_object, columns)
add_owners_to_saved_queries(metadata, tag, tagged_object, columns)
Expand Down Expand Up @@ -482,7 +482,7 @@ def add_favorites(metadata: MetaData) -> None:
insert = tag.insert()
for (id_,) in db.session.execute(ids):
with contextlib.suppress(IntegrityError): # already exists
db.session.execute(insert, name=f"favorited_by:{id_}", type=TagType.type)
db.session.execute(insert, name=f"favorited_by:{id_}", type=TagType.TYPE)
favstars = (
select(
[
Expand Down
16 changes: 8 additions & 8 deletions superset/daos/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def create_custom_tagged_objects(
clean_tag_names: set[str] = {tag.strip() for tag in tag_names}

for name in clean_tag_names:
type_ = TagType.custom
type_ = TagType.CUSTOM
tag = TagDAO.get_by_name(name, type_)
tagged_objects.append(
TaggedObject(object_id=object_id, object_type=object_type, tag=tag)
Expand Down Expand Up @@ -117,7 +117,7 @@ def delete_tags(tag_names: list[str]) -> None:
db.session.delete(tag)

@staticmethod
def get_by_name(name: str, type_: TagType = TagType.custom) -> Tag:
def get_by_name(name: str, type_: TagType = TagType.CUSTOM) -> Tag:
"""
returns a tag if one exists by that name, none otherwise.
important!: Creates a tag by that name if the tag is not found.
Expand Down Expand Up @@ -182,7 +182,7 @@ def get_tagged_objects_for_tags(
TaggedObject,
and_(
TaggedObject.object_id == Dashboard.id,
TaggedObject.object_type == ObjectType.dashboard,
TaggedObject.object_type == ObjectType.DASHBOARD,
),
)
.join(Tag, TaggedObject.tag_id == Tag.id)
Expand All @@ -192,7 +192,7 @@ def get_tagged_objects_for_tags(
results.extend(
{
"id": obj.id,
"type": ObjectType.dashboard.name,
"type": ObjectType.DASHBOARD.name,
"name": obj.dashboard_title,
"url": obj.url,
"changed_on": obj.changed_on,
Expand All @@ -212,7 +212,7 @@ def get_tagged_objects_for_tags(
TaggedObject,
and_(
TaggedObject.object_id == Slice.id,
TaggedObject.object_type == ObjectType.chart,
TaggedObject.object_type == ObjectType.CHART,
),
)
.join(Tag, TaggedObject.tag_id == Tag.id)
Expand All @@ -221,7 +221,7 @@ def get_tagged_objects_for_tags(
results.extend(
{
"id": obj.id,
"type": ObjectType.chart.name,
"type": ObjectType.CHART.name,
"name": obj.slice_name,
"url": obj.url,
"changed_on": obj.changed_on,
Expand All @@ -241,7 +241,7 @@ def get_tagged_objects_for_tags(
TaggedObject,
and_(
TaggedObject.object_id == SavedQuery.id,
TaggedObject.object_type == ObjectType.query,
TaggedObject.object_type == ObjectType.QUERY,
),
)
.join(Tag, TaggedObject.tag_id == Tag.id)
Expand All @@ -250,7 +250,7 @@ def get_tagged_objects_for_tags(
results.extend(
{
"id": obj.id,
"type": ObjectType.query.name,
"type": ObjectType.QUERY.name,
"name": obj.label,
"url": obj.url(),
"changed_on": obj.changed_on,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@

class ObjectType(enum.Enum):
# pylint: disable=invalid-name
query = 1
chart = 2
dashboard = 3
dataset = 4
QUERY = 1
CHART = 2
DASHBOARD = 3
DATASET = 4


# Define the tagged_object table structure
Expand Down
2 changes: 1 addition & 1 deletion superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class Dashboard(AuditMixinNullable, ImportExportMixin, Model):
overlaps="objects,tag,tags",
secondary="tagged_object",
primaryjoin="and_(Dashboard.id == TaggedObject.object_id, "
"TaggedObject.object_type == 'dashboard')",
"TaggedObject.object_type == 'DASHBOARD')",
secondaryjoin="TaggedObject.tag_id == Tag.id",
viewonly=True, # cascading deletion already handled by superset.tags.models.ObjectUpdater.after_delete
)
Expand Down
2 changes: 1 addition & 1 deletion superset/models/slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Slice( # pylint: disable=too-many-public-methods
secondary="tagged_object",
overlaps="objects,tag,tags",
primaryjoin="and_(Slice.id == TaggedObject.object_id, "
"TaggedObject.object_type == 'chart')",
"TaggedObject.object_type == 'CHART')",
secondaryjoin="TaggedObject.tag_id == Tag.id",
viewonly=True, # cascading deletion already handled by superset.tags.models.ObjectUpdater.after_delete
)
Expand Down
2 changes: 1 addition & 1 deletion superset/models/sql_lab.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ class SavedQuery(
secondary="tagged_object",
overlaps="objects,tag,tags",
primaryjoin="and_(SavedQuery.id == TaggedObject.object_id, "
"TaggedObject.object_type == 'query')",
"TaggedObject.object_type == 'QUERY')",
secondaryjoin="TaggedObject.tag_id == Tag.id",
viewonly=True, # cascading deletion already handled by superset.tags.models.ObjectUpdater.after_delete
)
Expand Down
31 changes: 8 additions & 23 deletions superset/tags/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
openapi_spec_methods_override,
TaggedObjectEntityResponseSchema,
TagGetResponseSchema,
TagPostBulkResponseSchema,
TagPostBulkSchema,
TagPostSchema,
TagPutSchema,
Expand Down Expand Up @@ -132,6 +133,8 @@ class TagRestApi(BaseSupersetModelRestApi):
openapi_spec_component_schemas = (
TagGetResponseSchema,
TaggedObjectEntityResponseSchema,
TagPostBulkResponseSchema,
TagPostBulkSchema,
)
apispec_parameter_schemas = {
"delete_tags_schema": delete_tags_schema,
Expand Down Expand Up @@ -211,40 +214,21 @@ def bulk_create(self) -> Response:
"""Bulk create tags and tagged objects
---
post:
summary: Get all objects associated with a tag
parameters:
- in: path
schema:
type: integer
name: tag_id
summary: Bulk create tags and tagged objects
requestBody:
description: Tag schema
required: true
content:
application/json:
schema:
type: object
properties:
tags:
description: list of tag names to add to object
type: array
items:
type: string
objects_to_tag:
description: list of object names to add to object
type: array
items:
type: array
$ref: '#/components/schemas/TagPostBulkSchema'
Comment on lines -214 to +224
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tags API and related schemas are in pretty rough shape. I would almost recommend doing a full refactor of these in some future version.

responses:
200:
description: Tag added to favorites
description: Bulk created tags and tagged objects
content:
application/json:
schema:
type: object
properties:
result:
type: object
$ref: '#/components/schemas/TagPostBulkResponseSchema'
302:
description: Redirects to the current digest
400:
Expand All @@ -267,6 +251,7 @@ def bulk_create(self) -> Response:
tagged_item: dict[str, Any] = self.add_model_schema.load(
{
"name": tag.get("name"),
"description": tag.get("description"),
"objects_to_tag": tag.get("objects_to_tag"),
}
)
Expand Down
4 changes: 2 additions & 2 deletions superset/tags/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ class UserCreatedTagTypeFilter(BaseFilter): # pylint: disable=too-few-public-me

def apply(self, query: Query, value: bool) -> Query:
if value:
return query.filter(Tag.type == TagType.custom)
return query.filter(Tag.type == TagType.CUSTOM)
if value is False:
return query.filter(Tag.type != TagType.custom)
return query.filter(Tag.type != TagType.CUSTOM)
return query


Expand Down
Loading
Loading