jscheffl commented on code in PR #61049:
URL: https://github.com/apache/airflow/pull/61049#discussion_r2726012043
##########
helm-tests/tests/helm_tests/other/test_hpa.py:
##########
@@ -106,10 +106,8 @@ def test_hpa_behavior(self, executor):
[
({"celery": {"persistence": {"enabled": True}}}, "StatefulSet"),
({"celery": {"persistence": {"enabled": False}}}, "Deployment"),
- ({"persistence": {"enabled": True}, "celery": {"persistence":
{"enabled": None}}}, "StatefulSet"),
- ({"persistence": {"enabled": False}, "celery": {"persistence":
{"enabled": None}}}, "Deployment"),
Review Comment:
Why are you removing these tests? Are they broken or not meaningful?
If disabled because of fix complexity this should keep a note at least.
##########
helm-tests/tests/helm_tests/airflow_core/test_scheduler.py:
##########
@@ -47,68 +47,17 @@ class TestScheduler:
"StatefulSet",
),
("LocalExecutor", {"celery": {"persistence": {"enabled": False}}},
"Deployment"),
- # Test workers.persistence.enabled flag when celery one is default
(expected no impact on kind)
+ # Test workers.persistence.enabled flag when celery one is default
("CeleryExecutor", {"persistence": {"enabled": False}},
"Deployment"),
("CeleryExecutor", {"persistence": {"enabled": True}},
"Deployment"),
("CeleryKubernetesExecutor", {"persistence": {"enabled": True}},
"Deployment"),
("CeleryExecutor,KubernetesExecutor", {"persistence": {"enabled":
True}}, "Deployment"),
("KubernetesExecutor", {"persistence": {"enabled": True}},
"Deployment"),
- ("LocalKubernetesExecutor", {"persistence": {"enabled": False}},
"StatefulSet"),
+ ("LocalKubernetesExecutor", {"persistence": {"enabled": False}},
"Deployment"),
("LocalKubernetesExecutor", {"persistence": {"enabled": True}},
"StatefulSet"),
("LocalExecutor", {"persistence": {"enabled": True}},
"StatefulSet"),
("LocalExecutor,KubernetesExecutor", {"persistence": {"enabled":
True}}, "StatefulSet"),
- ("LocalExecutor", {"persistence": {"enabled": False}},
"StatefulSet"),
- # Test workers.persistence.enabled flag when celery one is unset
Review Comment:
Oh, you delete a couple of tests. Can you add a comment why?
##########
helm-tests/tests/helm_tests/other/test_keda.py:
##########
@@ -179,10 +179,8 @@ def test_keda_query_kubernetes_queue(self, executor,
queue):
[
({"celery": {"persistence": {"enabled": True}}}, "StatefulSet"),
({"celery": {"persistence": {"enabled": False}}}, "Deployment"),
- ({"persistence": {"enabled": True}, "celery": {"persistence":
{"enabled": None}}}, "StatefulSet"),
- ({"persistence": {"enabled": False}, "celery": {"persistence":
{"enabled": None}}}, "Deployment"),
({"persistence": {"enabled": True}}, "StatefulSet"),
- ({"persistence": {"enabled": False}}, "StatefulSet"),
+ ({"persistence": {"enabled": False}}, "Deployment"),
Review Comment:
I see (similar like in my PR) you are switching some storage definitions -
just similar like in my parallel PR - was the test wrong and you needed to
correct to Helm Chart fixes or did you need to bend test to mask a problem?
--
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]
