-
Notifications
You must be signed in to change notification settings - Fork 15.6k
fix: Helm execution_api_server_url when base_url has a subpath #51454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Helm execution_api_server_url when base_url has a subpath #51454
Conversation
Can you add some tests with those examples you written? |
Sure thing, I initially missed the tests here: https://github.com/apache/airflow/blob/main/helm-tests%2Ftests%2Fhelm_tests%2Fairflow_core%2Ftest_api_server.py I will add a new test case today. |
@romsharon98 done, i add 4 cases for airflow > v3 and one for <v3. Let me know if there are other cases we want to include. |
6e0fbdf
to
9866707
Compare
The test failure seems unrelated
|
When the `config.core.base_url` has a subpath, this path should be copied to the `execution_api_server_url` as a path-prefix. For example, when `base_url=http://example.com/airflow`, then the `execution_api_server_url=http://name-api-server:8080/airflow/execution` The chart behavior is unchanged if - the `base_url` is empty - the `base_url` does _not_ have a subpath - the `execution_api_server_url` is already set in the config Signed-off-by: Lucas Roesler <[email protected]>
Signed-off-by: Lucas Roesler <[email protected]>
136e7fe
to
3ab8304
Compare
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
When the
config.core.base_url
has a subpath, this path should be copied to theexecution_api_server_url
as a path-prefix.For example, when
base_url=http://example.com/airflow
, then theexecution_api_server_url=http://name-api-server:8080/airflow/execution
The chart behavior is unchanged if
base_url
is emptybase_url
does not have a subpathexecution_api_server_url
is already set in the configThis can easily be tested using
helm template
, here are the 4 cases I consideredNo config overrides will produce the current behavior
Adding a
base_url
with a sub-path shows the new behavior of copying the subpath to the execution urlAdding a
base_url
without a subpath also preserves the existing behaviorFinallly, even adding a subpath to the base url, if the execution url is directly set, this is preserved
Closes: #51450