Test connections via workers instead of the API server#68113
Test connections via workers instead of the API server#68113anishgirianish wants to merge 1 commit into
Conversation
boring-cyborg
Bot
added
area:translations
area:UI
anishgirianish
requested review from
bbovenzi,
choo121600,
guan404ming,
pierrejeambrun,
ryanahamilton,
shubhamraj-git and
vatsrahul1001
as code owners
potiuk
added
the
ready for maintainer review
pierrejeambrun
left a comment
pierrejeambrun
left a comment
There was a problem hiding this comment.
Thanks for the PR, tested and working as expected.
I don't think we can do this before all executor support the async connection testing flow. Otherwise it will just break the feature for people using the celery executor for instance.
We also need to update the documentation, for instance here /Users/pierrejeambrun/Documents/astronomer/git-repositories/airflow-agents/agent-1/airflow-core/docs/howto/connection.rst,
When testing in the Airflow UI, the test executes from the webserver so this feature is subject to network egress rules setup for your webserver.
For security reasons, the test connection functionality is disabled by default across Airflow UI, API
Etc...
Also a few suggestions / comments.
| "testInProgress": { | ||
| "title": "A connection test is already in progress" | ||
| }, |
There was a problem hiding this comment.
Don't need to do this with a specific message and error handler.
Just use a common handler that will display the error message from the server
{
"detail": "An active connection test already exists for connection_id `test_failing`."
}
You can take a look at other similar hooks
| }; | ||
|
|
||
| const { isPending, mutate } = useTestConnection((result) => { | ||
| const { isPending, test } = useTestConnection((result) => { |
There was a problem hiding this comment.
mutate is fine it's the common convention in the code base.
| const { isPending, test } = useTestConnection((result) => { | |
| const { isPending, mutate } = useTestConnection((result) => { |
| toaster.create({ | ||
| description: res.message, | ||
| title: translate("connections.testError.title"), | ||
| title: | ||
| (error as ApiError).status === 409 | ||
| ? translate("connections.testInProgress.title") | ||
| : translate("connections.testError.title"), | ||
| type: "error", |
There was a problem hiding this comment.
You can reuse createErrorToaster.
| undefined, | ||
| { | ||
| enabled: token !== undefined, | ||
| refetchInterval: (query) => (isActive(query.state.data?.state) ? POLL_INTERVAL : false), |
There was a problem hiding this comment.
Refresh interval should come from useAutoRefresh. (To be consistent in timing with other request in the application)
| type ConnectionStatus = boolean | undefined; | ||
|
|
||
| const ACTIVE_STATES = new Set(["pending", "queued", "running"]); | ||
| const POLL_INTERVAL = 2000; |
pierrejeambrun
changed the title
3 participants
Was generative AI tooling used to co-author this PR?
Follow-up to #62343, which moved connection testing to workers but left the UI on the old sync endpoint.
This wires the Connections "Test" button to the new worker flow: enqueue the test, then poll until it finishes. The result shows up as a toast (with the worker's message) and the usual green/red icon. Clicking Test while one is already running shows an "already in progress" message.
related: #62343
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.Note
✅ Ready for review · @anishgirianish →
@potiuk· 2026-06-12 13:30 UTCThanks @anishgirianish — all checks are green and this PR is marked ready for maintainer review. The ball is with the maintainers now; a maintainer will take the next look.
Automated triage — may be imperfect.