Skip to content

Add Snowflake Tracking#3492

Merged
sungchun12 merged 7 commits intomainfrom
feat/snowflake-tracking
Dec 10, 2024
Merged

Add Snowflake Tracking#3492
sungchun12 merged 7 commits intomainfrom
feat/snowflake-tracking

Conversation

@sungchun12
Copy link
Contributor

@sungchun12 sungchun12 commented Dec 9, 2024

See how people are using SQLMesh with Snowflake. Should look and feel like this in the query history.
image

role: t.Optional[str] = None
authenticator: t.Optional[str] = None
token: t.Optional[str] = None
application: str = 'Tobiko_SQLMesh'
Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn’t be configurable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made it a property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, made it a classvar instead. Let me know if you want more protection on it.

role: t.Optional[str] = None
authenticator: t.Optional[str] = None
token: t.Optional[str] = None
application: t.ClassVar[str] = "Tobiko_SQLMesh"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think doing t.Literal["Tobiko_SQLMesh"] = "Tobiko_SQLMesh" is more common in the codebase, but not sure if t.ClassVar semantics match the use case better.

Does this guard from the field being mutated, though? Perhaps we should add it to _FIELD_UPDATE_STRATEGY with the IMMUTABLE strategy? cc @tobymao

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll assume Iaroslav's 👍 means it's worth changing this.

Copy link
Contributor Author

@sungchun12 sungchun12 Dec 10, 2024

Choose a reason for hiding this comment

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

I chose a ClassVar because it'll raise this error if you try to mutate on an instance of a class:

AttributeError: 'application' is a ClassVar of `SnowflakeConnectionConfig` and cannot be set on an instance. If you want to set a value on the class, use `SnowflakeConnectionConfig.application = value`.

@sungchun12 sungchun12 merged commit 3b404fe into main Dec 10, 2024
@sungchun12 sungchun12 deleted the feat/snowflake-tracking branch December 10, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants