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

openlineage,gcs: use proper name for openlineage methods #32956

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

mobuchowski
Copy link
Contributor

The GCSToGCSOperator uses wrong names for OpenLineage methods. Those should be get_openlineage_facets_on_complete.

BTW, maybe we should make it "official" interface in BaseOperator to prevent those mistakes.

Signed-off-by: Maciej Obuchowski <obuchowski.maciej@gmail.com>
@hussein-awala
Copy link
Member

BTW, maybe we should make it "official" interface in BaseOperator to prevent those mistakes.

Or create an interface (maybe multiple ones) to encapsulate all these methods, and add it as a super class for the operators/hooks classes.
In this case, you can replace method_exists("get_openlineage_facets_on_start") by isinstance(task, <interface>).

@potiuk
Copy link
Member

potiuk commented Jul 31, 2023

The GCSToGCSOperator uses wrong names for OpenLineage methods. Those should be get_openlineage_facets_on_complete.

BTW, maybe we should make it "official" interface in BaseOperator to prevent those mistakes.

Or create an interface (maybe multiple ones) to encapsulate all these methods, and add it as a super class for the operators/hooks classes.

Agree (@hussein-awala 's comment). That would also make it easier to see if the operator already is "supposed" to have Open-Lineage integration

@mobuchowski mobuchowski merged commit f9cddf3 into main Jul 31, 2023
42 checks passed
@Taragolis Taragolis deleted the fix-gcs-openlineage-impl branch September 26, 2023 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants