-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
openlineage, gcs: add openlineage methods for GcsToGcsOperator #31350
Conversation
fa7a3c0
to
ec0ae89
Compare
ec0ae89
to
49167ab
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 we should add documentation around this along with the following changes
- Add run facet which captures the error as per https://github.com/OpenLineage/OpenLineage/blob/main/spec/OpenLineage.md#run-facets
- Add test case for error scenario where runfacet is populated
- Add documentation
return None | ||
|
||
return OperatorLineage( | ||
inputs=[ |
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.
What about DataSource dataset facet as per https://github.com/OpenLineage/OpenLineage/blob/main/spec/OpenLineage.md#dataset-facets?
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.
is this not required @mobuchowski ?
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 don't think it's required here
In general case it's hard to capture errors now, since |
9af4382
to
edf62f9
Compare
65afad8
to
cf86fb7
Compare
26d29bb
to
0b83011
Compare
return None | ||
|
||
return OperatorLineage( | ||
inputs=[ |
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.
is this not required @mobuchowski ?
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.
Once test passes it should be good to be merged.
Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
0b83011
to
5158070
Compare
This PR adds OpenLineage support for GcsToGcsOperator.