Miretpl commented on code in PR #70425:
URL: https://github.com/apache/airflow/pull/70425#discussion_r3684600990
##########
helm-tests/tests/helm_tests/airflow_aux/test_basic_helm_chart.py:
##########
@@ -384,35 +384,41 @@ def test_network_policies_are_valid(self,
airflow_version, executor):
"flower": {"enabled": True},
},
("flower-ingress", "Ingress", "flower-ingress"),
+ ("ingress", "Ingress", "airflow-ingress"),
[],
- id="flower-ingress",
+ id="ingress",
),
pytest.param(
{
- "ingress": {
- "web": {"enabled": True},
- "apiServer": {"enabled": True},
- },
+ "ingress": {"apiServer": {"enabled": True}},
Review Comment:
> We're not removing the ingress.web option from chart. This parameterized
case is being changed to cover labels on the new Webserver HTTPRoute.
> Following the mutually exclusive routing behavior introduced for the API
server in https://github.com/apache/airflow/pull/68552 and already used for
Flower, only one the Webserver Ingress and HTTPRoute can be enabled. Keeping
ingress.web.enabled: true here together with webserver.htttpRoute.enabled: true
would intentionally fail template rendering.
Out of context. Looking at the context (particular line to which I post
feedback) it was the question "why we are removing the test case for the web
option if it is still supported in the 1.2x line?". I guess AI treated it like
it would be in the values file or sth and that is why this miscontext
originated.
> The Webserver Ingress remains covered by the preceding ingress case and by
test_ingress_web.py.
Also, kinda, out of context.
---
IMO, this should not be changed. It validates the deprecated path for 2.11
version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
