Skip to content

Conversation

@eladkal
Copy link
Contributor

@eladkal eladkal commented Jul 1, 2023

Providers wave for July.

open issues:

@potiuk
Copy link
Member

potiuk commented Jul 2, 2023

Rebased after Pydantic < 2.0.0 limit merged

@eladkal
Copy link
Contributor Author

eladkal commented Jul 2, 2023

Rebased after Pydantic < 2.0.0 limit merged

Thanks! The release wave is still on hold till we figure out the open issue of common.sql

@potiuk
Copy link
Member

potiuk commented Jul 2, 2023

Thanks! The release wave is still on hold till we figure out the open issue of common.sql

Which one ?

@eladkal
Copy link
Contributor Author

eladkal commented Jul 2, 2023

Which one ?

The OL extra added to common.sql
#31398 (comment)

@potiuk
Copy link
Member

potiuk commented Jul 2, 2023

As explained in #31398, I think there are fundamentally no problems with releasing the change with openlineage extra.

Our tests are passing (which also means that there are no imports that will not work becasue of openlineage missing (currently we do not install openlineage provider during tests) - so we have quite a certainty that not having openlineage provider is going to work fine for common.sql

@eladkal
Copy link
Contributor Author

eladkal commented Jul 3, 2023

As explained in #31398, I think there are fundamentally no problems with releasing the change with openlineage extra.

Our tests are passing (which also means that there are no imports that will not work becasue of openlineage missing (currently we do not install openlineage provider during tests) - so we have quite a certainty that not having openlineage provider is going to work fine for common.sql

I think releasing an extra that you can't install is not like installing a provider with a feature that isn't useable yet and the code tells you that you can't use it (like we did with Notifiers being released in providers before we released it in core) but If others consider it as OK I will proceed with the release

@eladkal eladkal force-pushed the providers branch 2 times, most recently from 8931922 to c782ed9 Compare July 3, 2023 07:58
@potiuk
Copy link
Member

potiuk commented Jul 3, 2023

I think releasing an extra that you can't install is not like installing a provider with a feature that isn't useable yet and the code tells you that you can't use it (like we did with Notifiers being released in providers before we released it in core) but If others consider it as OK I will proceed with the release

I think we won't avoid it at times - sometimes we even deliberately released something that was not usable so that we could refer to it in the future. Not ideal, but this is sometimes inevitable when we have two independent things being developed/released when usage of one will depend on the other.

But if it bothers you, I can draft a very small PR to exclude it.

@potiuk
Copy link
Member

potiuk commented Jul 3, 2023

PR: here: #32325

You will have to regenerate the docs (specficially common.sql) after merging and rebasing on top of this one @eladkal

@eladkal
Copy link
Contributor Author

eladkal commented Jul 4, 2023

After discussing with @potiuk I'll continue with the release as is

@eladkal
Copy link
Contributor Author

eladkal commented Jul 4, 2023

Looks like there is issue with MsSQL tests

@potiuk
Copy link
Member

potiuk commented Jul 4, 2023

Looks like there is issue with MsSQL tests

Likely #32343 should fix main ( guess it's the sqlalchemy refactor).

@eladkal eladkal merged commit cb4927a into apache:main Jul 5, 2023
@eladkal eladkal deleted the providers branch July 5, 2023 07:12
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.

2 participants