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.

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

Jump to bottom

[CT-2684] NodeType strings should not contain spaces #7841

Closed
jtcohen6 opened this issue Jun 12, 2023 · 1 comment · Fixed by #7842 or #7947
Closed

[CT-2684] NodeType strings should not contain spaces #7841

jtcohen6 opened this issue Jun 12, 2023 · 1 comment · Fixed by #7842 or #7947
Assignees
@peterallenwebb
Labels
tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality

Comments

@jtcohen6
Copy link
Contributor

The string representations of NodeType enum should not contain spaces. They should be snake case, or ideally a single word.

This affects how we construct the node's unique_id, which also affects its key in the manifest, how it appears in structured logging, etc.

This is currently relevant for "SQL operations" (a.k.a. "inline" queries) and "semantic models."

dbt-core/core/dbt/node_types.py

Line 29 in 6f51de4

SqlOperation = "sql operation"

dbt-core/core/dbt/node_types.py

Line 36 in 6f51de4

SemanticModel = "semantic model"

@jtcohen6 jtcohen6 added the tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality label Jun 12, 2023
@github-actions github-actions bot changed the title NodeType strings should not contain spaces [CT-2684] NodeType strings should not contain spaces Jun 12, 2023
This was referenced Jun 12, 2023
Merged
Merged
@peterallenwebb
Copy link
Contributor

Yeah, I was wondering about this even when I did it. Since we are following up a bunch of Sematnic Interfaces issues right now, I'll add this to the list.

@peterallenwebb peterallenwebb self-assigned this Jun 12, 2023
@peterallenwebb peterallenwebb added tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality and removed tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality labels Jun 12, 2023
@jtcohen6 jtcohen6 mentioned this issue Jun 26, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@peterallenwebb peterallenwebb

Labels
tech_debt Behind-the-scenes changes, with little direct impact on end-user functionality
Projects
None yet
Milestone
No milestone
2 participants
@peterallenwebb @jtcohen6