Skip to content

Conversation

@gopidesupavan
Copy link
Member

@gopidesupavan gopidesupavan commented Nov 14, 2024

Version imports causing issues when using from init : #44011
#44011 (comment)


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@eladkal
Copy link
Contributor

eladkal commented Nov 14, 2024

cc @potiuk

@potiuk
Copy link
Member

potiuk commented Nov 14, 2024

This is strange error

@gopidesupavan
Copy link
Member Author

@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label Nov 14, 2024
@potiuk potiuk closed this Nov 14, 2024
@potiuk potiuk reopened this Nov 14, 2024
@potiuk
Copy link
Member

potiuk commented Nov 14, 2024

Applying full tests needed should help

@potiuk
Copy link
Member

potiuk commented Nov 14, 2024

The problem is that "sdist" test build new providers and try to install them, and they do it in "chunks" - and only for providers affected in this PR. This is a bit missing piece - this PR only modifies the standard provider but no common.sql 1.20 that it depends on, so it misses 1.20 version locally built.

We could likely solve it by smarter selection which providers should be built for sdist builds, but for now "full tests needed" should be enough - also this will go away after we merge this one an release common.sql 1.20 , and applying "full tests needed" should solve the problem as all provider's sdist will be built.

@eladkal
Copy link
Contributor

eladkal commented Nov 14, 2024

We could likely solve it by smarter selection which providers should be built for sdist builds, but for now "full tests needed" should be enough - also this will go away after we merge this one an release common.sql 1.20 , and applying "full tests needed" should solve the problem as all provider's sdist will be built.

Yeah the problem should not happen after merging to main.
We can open a followup task in Github issue to get it done when we have the time

@potiuk
Copy link
Member

potiuk commented Nov 14, 2024

We can open a followup task in Github issue to get it done when we have the time

Yeah. I was adding it as you wrote it :) #44023

@gopidesupavan
Copy link
Member Author

The problem is that "sdist" test build new providers and try to install them, and they do it in "chunks" - and only for providers affected in this PR. This is a bit missing piece - this PR only modifies the standard provider but no common.sql 1.20 that it depends on, so it misses 1.20 version locally built.

We could likely solve it by smarter selection which providers should be built for sdist builds, but for now "full tests needed" should be enough - also this will go away after we merge this one an release common.sql 1.20 , and applying "full tests needed" should solve the problem as all provider's sdist will be built.

oh i see, good learning bit to know this :)

@gopidesupavan
Copy link
Member Author

tests are failing for openlineage, created this #44025

@eladkal eladkal merged commit 2ef8438 into apache:main Nov 14, 2024
@gopidesupavan gopidesupavan deleted the fix-version-imports branch November 14, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers full tests needed We need to run full set of tests for this PR to merge provider:standard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants