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

Rename relation identifier to table name #25400

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

clairelin135
Copy link
Contributor

@clairelin135 clairelin135 commented Oct 21, 2024

Summary & Motivation

Rename relation identifier to table name to increase clarity. Context here.

Does a direct rename to switch references from relation_identifier to table_name, since integration packages should have the same version as the dagster package.

In the UI, to handle backcompat, display metadata with either key.

How I Tested These Changes

BK

Changelog

Renames the dagster/relation_identifier metadata key to dagster/table_name.

@clairelin135
Copy link
Contributor Author

clairelin135 commented Oct 21, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @clairelin135 and the rest of your teammates on Graphite Graphite

Copy link

github-actions bot commented Oct 21, 2024

Deploy preview for dagster-docs-beta ready!

Preview available at https://dagster-docs-beta-jbtv2ly29-elementl.vercel.app

Direct link to changed pages:

@clairelin135 clairelin135 force-pushed the 10-21-claire/relation-identifier-to-table-name branch 2 times, most recently from fe4ac9f to edd0a89 Compare October 21, 2024 18:21
Copy link

github-actions bot commented Oct 21, 2024

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-ameweihho-elementl.vercel.app
https://10-21-claire-relation-identifier-to-table-name.core-storybook.dagster-docs.io

Built with commit b1dbce9.
This pull request is being automatically deployed with vercel-action

@clairelin135 clairelin135 marked this pull request as ready for review October 21, 2024 18:39
@graphite-app graphite-app bot added the area: docs Related to documentation in general label Oct 21, 2024
m: MetadataEntryLabelOnly,
): m is TextMetadataEntry => m && m.label === 'dagster/relation_identifier';
export const isCanonicalTableNameEntry = (m: MetadataEntryLabelOnly): m is TextMetadataEntry =>
m && (m.label === 'dagster/relation_identifier' || m.label === 'dagster/table_name');
Copy link
Contributor

Choose a reason for hiding this comment

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

Smart to include both!

Copy link
Contributor

@cmpadden cmpadden left a comment

Choose a reason for hiding this comment

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

This looks good to me.

Confirmed that no case insensitive relation_identifier or relation identifier exists in the monorepo apart from a couple of comments.

There are some programmatic names, like RelationKey.identifier in dagster-dbt but these are not forward facing.

Copy link
Member

@benpankow benpankow left a comment

Choose a reason for hiding this comment

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

🙌

@clairelin135 clairelin135 force-pushed the 10-21-claire/relation-identifier-to-table-name branch from 12c741a to b1dbce9 Compare October 23, 2024 16:36
@clairelin135 clairelin135 merged commit 64e0e4b into master Oct 23, 2024
4 of 5 checks passed
@clairelin135 clairelin135 deleted the 10-21-claire/relation-identifier-to-table-name branch October 23, 2024 17:07
Grzyblon pushed a commit to Grzyblon/dagster that referenced this pull request Oct 26, 2024
## Summary & Motivation

Rename relation identifier to table name to increase clarity. [Context
here](https://dagsterlabs.slack.com/archives/C06AWMPT3DF/p1729269974738339).

Does a direct rename to switch references from `relation_identifier` to
`table_name`, since integration packages should have the same version as
the dagster package.

In the UI, to handle backcompat, display metadata with either key.

## How I Tested These Changes
BK

## Changelog
Renames the `dagster/relation_identifier` metadata key to
`dagster/table_name`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Related to documentation in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants