-
Notifications
You must be signed in to change notification settings - Fork 56
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
flow: x-schema-name annotation for 2nd-to-last resource path #1572
Conversation
a6725df
to
b4b9bd8
Compare
b4b9bd8
to
5afcb73
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is looking directionally right. I left some inline comments, and just wanted to add a general request to rename any collection_name
or task_name
variables that aren't valid catalog names, since I didn't leave separate comments on each one.
c00ffa6
to
ca6988e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few more comments, but I think this is getting pretty close.
70c9d98
to
1c40d8b
Compare
3594a3d
to
7c12245
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remaining issue that I'm seeing is just how to handle the possibility of mismatch between the target_schema
and delta_updates
fields on the materialization spec, and the resource spec schema of the materialization connector. To me, it seems worth validating that the connector actually has the necessary annotations on the resource spec schema. But it might be worth discussing as others may disagree.
56bb981
to
9b298a3
Compare
7d2e5c7
to
45643e3
Compare
45643e3
to
ea8ddda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few more comments/questions
ea1cf09
to
6def9e5
Compare
6def9e5
to
4a2b7a2
Compare
94454da
to
cbac742
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description:
Workflow steps:
Documentation links affected:
(list any documentation links that you created, or existing ones that you've identified as needing updates, along with a brief description)
Notes for reviewers:
(anything that might help someone review this PR)
This change is