dstandish commented on a change in pull request #14011:
URL: https://github.com/apache/airflow/pull/14011#discussion_r570679382
##########
File path: chart/tests/test_flower.py
##########
@@ -22,12 +22,21 @@
from tests.helm_template_generator import render_chart
-class FlowerAuthorizationTest(unittest.TestCase):
+class FlowerTest(unittest.TestCase):
Review comment:
given that airflow uses pytest shouldn't the class be named TestFlower?
##########
File path: chart/tests/test_flower.py
##########
@@ -22,12 +22,21 @@
from tests.helm_template_generator import render_chart
-class FlowerAuthorizationTest(unittest.TestCase):
+class FlowerTest(unittest.TestCase):
+ def test_should_create_flower_by_celery_executor(self):
+ docs = render_chart(
+ values={"executor": "CeleryExecutor", "flower": {"enabled": True}},
+ show_only=["templates/flower/flower-deployment.yaml"],
+ )
+
+ assert "RELEASE-NAME-flower" == jmespath.search("metadata.name",
docs[0])
+ assert "flower" ==
jmespath.search("spec.template.spec.containers[0].name", docs[0])
+
def test_should_create_flower_deployment_with_authorization(self):
docs = render_chart(
values={
"executor": "CeleryExecutor",
- "flower": {"username": "flower", "password": "fl0w3r"},
+ "flower": {"enabled": True, "username": "flower", "password":
"fl0w3r"},
Review comment:
perhaps it makes sense to not specify "enabled" here, to test what
happens by default
##########
File path: chart/tests/test_flower.py
##########
@@ -22,12 +22,21 @@
from tests.helm_template_generator import render_chart
-class FlowerAuthorizationTest(unittest.TestCase):
+class FlowerTest(unittest.TestCase):
+ def test_should_create_flower_by_celery_executor(self):
+ docs = render_chart(
+ values={"executor": "CeleryExecutor", "flower": {"enabled": True}},
Review comment:
you think it is worth parameterizing to confim that it is created if and
only if enabled?
##########
File path: chart/values.yaml
##########
@@ -468,6 +468,9 @@ webserver:
# Flower settings
flower:
+ # Enable flower.
+ # If Ture, and using CeleryExecutor/CeleryKubernetesExecutor, will deploy
flow ui.
Review comment:
spelling
##########
File path: chart/values.yaml
##########
@@ -468,6 +468,9 @@ webserver:
# Flower settings
flower:
+ # Enable flower.
Review comment:
i think you can chop this line because 472 already does a better job of
explaining
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
