Skip to content

Conversation

@Lee-W
Copy link
Member

@Lee-W Lee-W commented Oct 2, 2024


^ 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.

@gopidesupavan
Copy link
Member

Ah ryt I have added this into bash operator there were tests failing in the pr due to standard procedure not installing, the pr is in review.

@Lee-W Lee-W force-pushed the temp-wordaround-for-Basic-tests-Test-OpenAPI-client branch from 0af9887 to 329c8f4 Compare October 2, 2024 08:45
@gopidesupavan
Copy link
Member

#42506 this has changes to provider manager and bash operator changes, in my opinion it breaks k8s tests also as there is import being used in celery executor utils . Because standard provider not getting installing in the container. It would be better to revert the changes ?
Cc: @romsharon98

@Lee-W
Copy link
Member Author

Lee-W commented Oct 2, 2024

for short term solution, we probably could go with this one. for long term, we will need to add it as fab dep and wait for its release

@Lee-W Lee-W marked this pull request as ready for review October 2, 2024 09:01
@Lee-W Lee-W requested review from ashb, kaxil and potiuk as code owners October 2, 2024 09:01
@gopidesupavan
Copy link
Member

here when bash operator gets imported this would fail because bash operator has imports related to hook? It's like dead lock.

@gopidesupavan
Copy link
Member

for short term solution, we probably could go with this one. for long term, we will need to add it as fab dep and wait for its release

Agree. API client works with this change , but my hunch is on k8s test might fail.

@gopidesupavan
Copy link
Member

And docker tests as well... As a workaround I have added skipif here #42252. But don't like skipping these tests not good. It would be better to release the standard provider basic version and installing it default provider lists.

@gopidesupavan
Copy link
Member

Created revert pr #42659

@Lee-W
Copy link
Member Author

Lee-W commented Oct 2, 2024

Ok, then let's use the revert commit and close this one. Thanks @gopidesupavan !

@Lee-W Lee-W closed this Oct 2, 2024
@potiuk
Copy link
Member

potiuk commented Oct 2, 2024

See the discussion in https://lists.apache.org/thread/1gshy5cjmp9wz5v8dozyh64jj3dyn5s4 - I think we should start releasing ".dev*" standard provider to PyPI manually to overcome some of the chicken-egg problems experienced here and avoiding the neeed for "skip-if"

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.

3 participants