Skip to content

Change enum on history events to str #1379

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.

Sign up for GitHub

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

Draft
samredai wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Change enum on history events to str #1379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b11bdad
Change enum on history events to str
samredai May 14, 2025
8083a48
lint
samredai May 14, 2025
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
102 changes: 102 additions & 0 deletions ...erver/alembic/versions/2025_05_14_1545-cb7368a4bfae_change_enum_columns_to_string_type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
"""Change enum columns to string type
Revision ID: cb7368a4bfae
Revises: 51547dcccb10
Create Date: 2025-05-14 15:45:43.734154+00:00
"""
# pylint: disable=no-member, invalid-name, missing-function-docstring, unused-import, no-name-in-module

import sqlalchemy as sa
from alembic import op
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = "cb7368a4bfae"
down_revision = "51547dcccb10"
branch_labels = None
depends_on = None


def upgrade():
with op.batch_alter_table("history", schema=None) as batch_op:
batch_op.alter_column(
"entity_type",
existing_type=postgresql.ENUM(
Copy link
Contributor

@shangyian shangyian May 14, 2025

Choose a reason for hiding this comment

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

Should this also drop the enum types from Postgres? Maybe with something like:

batch_op.execute("DROP TYPE IF EXISTS entitytype")
batch_op.execute("DROP TYPE IF EXISTS activitytype")

"ATTRIBUTE",
"AVAILABILITY",
"BACKFILL",
"CATALOG",
"COLUMN_ATTRIBUTE",
"DEPENDENCY",
"ENGINE",
"LINK",
"MATERIALIZATION",
"NAMESPACE",
"NODE",
"PARTITION",
"QUERY",
"TAG",
name="entitytype",
),
type_=sa.String(length=20),
existing_nullable=True,
)
batch_op.alter_column(
"activity_type",
existing_type=postgresql.ENUM(
"CREATE",
"DELETE",
"RESTORE",
"UPDATE",
"REFRESH",
"TAG",
"SET_ATTRIBUTE",
"STATUS_CHANGE",
name="activitytype",
),
type_=sa.String(length=20),
existing_nullable=True,
)


def downgrade():
with op.batch_alter_table("history", schema=None) as batch_op:
batch_op.alter_column(
"activity_type",
existing_type=sa.String(length=20),
type_=postgresql.ENUM(
"CREATE",
"DELETE",
"RESTORE",
"UPDATE",
"REFRESH",
"TAG",
"SET_ATTRIBUTE",
"STATUS_CHANGE",
name="activitytype",
),
existing_nullable=True,
)
batch_op.alter_column(
"entity_type",
existing_type=sa.String(length=20),
type_=postgresql.ENUM(
"ATTRIBUTE",
"AVAILABILITY",
"BACKFILL",
"CATALOG",
"COLUMN_ATTRIBUTE",
"DEPENDENCY",
"ENGINE",
"LINK",
"MATERIALIZATION",
"NAMESPACE",
"NODE",
"PARTITION",
"QUERY",
"TAG",
name="entitytype",
),
existing_nullable=True,
)
10 changes: 4 additions & 6 deletions datajunction-server/datajunction_server/database/history.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@
JSON,
BigInteger,
DateTime,
Enum,
Index,
Integer,
String,
)
from sqlalchemy.orm import Mapped, mapped_column

from datajunction_server.database.base import Base
from datajunction_server.internal.history import ActivityType, EntityType
from datajunction_server.typing import UTCDatetime


Expand All @@ -35,14 +33,14 @@ class History(Base):
BigInteger().with_variant(Integer, "sqlite"),
primary_key=True,
)
entity_type: Mapped[Optional[EntityType]] = mapped_column(
Enum(EntityType),
entity_type: Mapped[Optional[str]] = mapped_column(
String(20),
Copy link
Member

@agorajek agorajek May 14, 2025

Choose a reason for hiding this comment

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

Why only 20?

Copy link
Contributor Author

@samredai samredai May 14, 2025

Choose a reason for hiding this comment

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

No reason, I just thought that the postgres default of infinite length seemed unnecessary for what will just be a string representation of an enum value. Should I make this larger? Or maybe don't specify the length at all?

default=None,
)
entity_name: Mapped[Optional[str]] = mapped_column(String, default=None)
node: Mapped[Optional[str]] = mapped_column(String, default=None)
activity_type: Mapped[Optional[ActivityType]] = mapped_column(
Enum(ActivityType),
activity_type: Mapped[Optional[str]] = mapped_column(
String(20),
default=None,
)
user: Mapped[Optional[str]] = mapped_column(String, default=None)
Expand Down