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, docs: tips for OpenLineage method implementation #31817

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

mobuchowski
Copy link
Contributor

Add tips for provider contributors on how to effectively add OpenLineage support.

Copy link
Member

@pankajastro pankajastro left a comment

Choose a reason for hiding this comment

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

LGTM

@pankajkoti pankajkoti self-requested a review June 13, 2023 06:23
How to properly implement OpenLineage methods?
==============================================

There are several things worth noting when implementing OpenLineage in operators.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
There are several things worth noting when implementing OpenLineage in operators.
There are a couple of things worth noting when implementing OpenLineage in operators.

I was anticipating a third thing when reading this, but there seems to be only two?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I was referring to everything below that line. I guess that's unclear, tried to fix this.

Copy link
Collaborator

@sunank200 sunank200 left a comment

Choose a reason for hiding this comment

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

A section would be useful about how one should add tests (both unit and system tests)


First, do not import OpenLineage methods on top-level, but in OL method itself.
This allows users to use your provider even if they do not have OpenLineage provider installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we add an example here (or) link to an already existing example? The example will be useful for folks who are starting out on a new OL implementation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added example from GcsToGcsOperator - due to it's simplicity.

@mobuchowski
Copy link
Contributor Author

@sunank200 added section on writing tests. I assume system test PR will be merged in similar shape it's today, if not I'll update the section.

@mobuchowski mobuchowski merged commit 3d8e214 into apache:main Jun 28, 2023
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.

6 participants