Skip to content

Conversation

@dabla
Copy link
Contributor

@dabla dabla commented Nov 6, 2024

This PR is linked to the original PR Introduce notion of dialects in DbApiHook, but only contains a small refactoring in which the sql handlers of the common-sql -provider are moved from the sql module to a dedicated handler module, as this will be needed later on to avoid circular import issues. This is to make the review easier as asked by @potiuk. This PR only impacts the common-sql provider, also a dedicated unit test has been added for the handlers.


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

@dabla
Copy link
Contributor Author

dabla commented Nov 7, 2024

Some things are failing but dunno why

@potiuk
Copy link
Member

potiuk commented Nov 7, 2024

It seems like installing node to build packages is very unstable recently (connection reset by peer etc.) - we will get it cached soon WIP is here: #43329 cc: @bugraoz93 that should improve stability.

@potiuk
Copy link
Member

potiuk commented Nov 7, 2024

For now rebasing, commit--amend or close/reopening the PR should retry it.

@dabla
Copy link
Contributor Author

dabla commented Nov 7, 2024

It seems like installing node to build packages is very unstable recently (connection reset by peer etc.) - we will get it cached soon WIP is here: #43329 cc: @bugraoz93 that should improve stability.

Isn't there a way to be able to re-run a failing step in the ci cd checks of github?

@dabla
Copy link
Contributor Author

dabla commented Nov 7, 2024

For now rebasing, commit--amend or close/reopening the PR should retry it.

Yeah just did it again, I was a bit worried cause my other PR didn't suffer from those random errors so I though maybe something was wrong in this one.

@bugraoz93
Copy link
Contributor

For now rebasing, commit--amend or close/reopening the PR should retry it.

Yeah just did it again, I was a bit worried cause my other PR didn't suffer from those random errors so I though maybe something was wrong in this one.

This is purely too many trials on node servers trying to download the dependencies and appear in random CI runs

@potiuk potiuk merged commit 7a2fae0 into apache:main Nov 19, 2024
@potiuk
Copy link
Member

potiuk commented Nov 19, 2024

Nice!

@dabla
Copy link
Contributor Author

dabla commented Nov 19, 2024

Nice!

Thx @potiuk

potiuk added a commit to potiuk/airflow that referenced this pull request Feb 27, 2025
Common.sql self-deprecated itself after apache#43747 by importing handlers
from the old location.
potiuk added a commit to potiuk/airflow that referenced this pull request Feb 27, 2025
Common.sql self-deprecated itself after apache#43747 by importing handlers
from the old location.
potiuk added a commit to potiuk/airflow that referenced this pull request Feb 27, 2025
Common.sql self-deprecated itself after apache#43747 by importing handlers
from the old location.
potiuk added a commit that referenced this pull request Feb 27, 2025
Common.sql self-deprecated itself after #43747 by importing handlers
from the old location.
Sharashchandra pushed a commit to Sharashchandra/airflow that referenced this pull request Feb 28, 2025
Common.sql self-deprecated itself after apache#43747 by importing handlers
from the old location.
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
Common.sql self-deprecated itself after apache#43747 by importing handlers
from the old location.
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.

5 participants