Skip to content

af: strip query string from airflow_url so version detection works#210

Merged
jlaneve merged 1 commit intomainfrom
af-strip-url-query
May 5, 2026
Merged

af: strip query string from airflow_url so version detection works#210
jlaneve merged 1 commit intomainfrom
af-strip-url-query

Conversation

@schnie
Copy link
Copy Markdown
Member

@schnie schnie commented May 4, 2026

Summary

  • Bug report: an Astro user got Failed to detect Airflow version at https://…/<deploymentid>?orgId=org_…. Ensure Airflow is running and accessible. from af config version (and every other af command). Reproduced against an Astro deployment by appending ?orgId=… to the stored URL.
  • Root cause: metadata.webserver_url from astro deployment inspect came back with a ?orgId=… query string for that deployment. We stored it as-is, then built API URLs via f"{airflow_url}/api/v2/version" — the result left the path at /dep and pushed /api/... into the query string. Server returned a 200/HTML redirect, JSON parse failed, and detect_version's bare except Exception: pass swallowed it.
  • Fix: normalize the base URL (strip query/fragment/trailing slash) at three boundaries — detect_version on entry, the adapter base __init__, and AstroDeployment.from_inspect_yaml during discovery. Existing bad configs keep working without forcing users to re-discover.
  • Also: stop swallowing probe failures silently. The RuntimeError from detect_version now includes the actual status code (or exception) from each probe, so the next opaque report won't be a black box.

Test plan

  • New unit test: detect_version strips a ?orgId=… query string before probing and returns the version.
  • New unit test: detect_version surfaces actual HTTP status codes in the failure message.
  • New unit test: AirflowV2Adapter.__init__ normalizes a URL with a query string.
  • New unit test: AstroDeployment.from_inspect_yaml strips the query string from webserver_url.
  • New unit tests: normalize_airflow_url handles query, fragment, trailing slash, empty, clean URLs.
  • Full pytest suite (566 tests) passes.
  • End-to-end repro: with a config containing ?orgId=…, af config version previously returned "Failed to detect Airflow version at …" and now returns the actual version JSON.

🤖 Generated with Claude Code

Stored URLs with a query string (eg the ?orgId=… that some Astro
deployments include in metadata.webserver_url) silently broke every
API call: building URLs via f"{airflow_url}/api/v2/version" left the
path at /dep and pushed /api/... into the query string. The 200/HTML
or redirect that came back was swallowed by detect_version's bare
except, surfacing only "Failed to detect Airflow version".

Normalize the base URL (strip query, fragment, trailing slash) at
three boundaries so existing bad configs keep working without
re-discovery: detect_version on entry, the adapter base __init__,
and AstroDeployment.from_inspect_yaml during discovery. Also surface
the actual probe failure (status code or exception) in the
RuntimeError so the next opaque report isn't a black box.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@jlaneve jlaneve merged commit fd6dd46 into main May 5, 2026
10 checks passed
@jlaneve jlaneve deleted the af-strip-url-query branch May 5, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants