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

CT 1443 remove root path #6172

Merged
merged 12 commits into from
Nov 4, 2022
Merged

CT 1443 remove root path #6172

merged 12 commits into from
Nov 4, 2022

Conversation

gshank
Copy link
Contributor

@gshank gshank commented Oct 28, 2022

resolves #6171

Description

As part of cleanup for the structured logging effort, remove the 'root_path' field from most nodes.

Checklist

@gshank gshank requested a review from a team October 28, 2022 14:47
@gshank gshank requested review from a team as code owners October 28, 2022 14:47
@cla-bot cla-bot bot added the cla:yes label Oct 28, 2022
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@gshank gshank removed the request for review from iknox-fa October 28, 2022 14:50
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

A few schema comments

@@ -37,12 +37,12 @@
},
"dbt_version": {
"type": "string",
"default": "1.2.0a1"
"default": "1.4.0a1"
Copy link
Member

Choose a reason for hiding this comment

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

It looks like only the default and description fields are changing. Do we really need to commit this?

Comment on lines 369 to 371
"root_path": {
"type": "string"
},
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to bump the schema to v8 instead of modifying?

@gshank gshank force-pushed the ct-1443-remove_root_path branch from a5a4aa4 to 849523c Compare November 1, 2022 18:04
@gshank
Copy link
Contributor Author

gshank commented Nov 4, 2022

Yes, we did need to bump the schema to v8. Have done that work.

@gshank gshank requested a review from emmyoop November 4, 2022 15:46
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

The run-results schema is just updating defaults and descriptions so run-results/v4.json should be removed from the PR.

@gshank
Copy link
Contributor Author

gshank commented Nov 4, 2022

Yeah, I've removed that run-results file a couple of times and then I run the script again... Gone now, I hope.

@gshank gshank requested a review from emmyoop November 4, 2022 18:28
@gshank gshank force-pushed the ct-1443-remove_root_path branch from f178fab to d83c734 Compare November 4, 2022 20:07
@gshank gshank merged commit 6c76137 into main Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-1443] Remove 'root_path' from most node definitions
2 participants