Copilot commented on code in PR #65030:
URL: https://github.com/apache/airflow/pull/65030#discussion_r3066476898
##########
helm-tests/tests/helm_tests/airflow_core/test_worker.py:
##########
@@ -2316,20 +2354,37 @@ def test_should_be_disabled_on_keda_enabled(self,
workers_values):
assert len(docs) == 1
- def test_should_add_component_specific_labels(self):
+ @pytest.mark.parametrize(
+ "workers_values",
+ [
+ {"celery": {"hpa": {"enabled": True}}, "labels": {"test_label":
"test_label_value"}},
+ {
+ "celery": {
+ "hpa": {"enabled": True},
+ "labels": {"test_label": "test_label_value"},
+ }
+ },
+ {
+ "labels": {"label": "value"},
+ "celery": {
+ "hpa": {"enabled": True},
+ "labels": {"test_label": "test_label_value"},
+ },
+ },
+ ],
+ )
+ def test_should_add_component_specific_labels(self, workers_values):
docs = render_chart(
values={
"executor": "CeleryExecutor",
- "workers": {
- "celery": {"hpa": {"enabled": True}},
- "labels": {"test_label": "test_label_value"},
- },
+ "workers": workers_values,
},
show_only=["templates/workers/worker-hpa.yaml"],
)
- assert "test_label" in jmespath.search("metadata.labels", docs[0])
- assert jmespath.search("metadata.labels", docs[0])["test_label"] ==
"test_label_value"
+ labels = jmespath.search("metadata.labels", docs[0])
+ assert labels["test_label"] == "test_label_value"
+ assert labels.get("labels") is None
Review Comment:
This assertion checks for a key named `labels`, but the parametrized input
uses a label key named `label` (e.g., `\"labels\": {\"label\": \"value\"}`).
This makes the test ineffective at catching leaked deprecated labels. Update
the assertion to check for `label` instead of `labels`.
##########
chart/docs/customizing-labels.rst:
##########
@@ -56,11 +56,6 @@ For example, to add specific labels to different components:
labels:
role: scheduler
- # Worker specific labels
- workers:
- labels:
- role: worker
-
# API Server specific labels
apiServer:
Review Comment:
The worker-specific labels example was removed, but the docs do not show the
new recommended configuration (`workers.celery.labels` and
`workers.kubernetes.labels`). Add an updated worker example demonstrating both
keys (and optionally mention `workers.labels` is deprecated) so users can
migrate without needing to infer the new structure.
##########
chart/docs/customizing-labels.rst:
##########
@@ -42,7 +42,7 @@ You can also set specific labels for individual Airflow
components, which will b
If the same label key exists in both global and component-specific labels, the
component-specific value takes precedence (overrides the global value).
This allows you to customize labels for specific components while still
maintaining common global labels across all resources.
-For example, to add specific labels to different components:
+For example, to add specific labels to different components like scheduler or
api-server:
.. code-block:: yaml
:caption: values.yaml
Review Comment:
The worker-specific labels example was removed, but the docs do not show the
new recommended configuration (`workers.celery.labels` and
`workers.kubernetes.labels`). Add an updated worker example demonstrating both
keys (and optionally mention `workers.labels` is deprecated) so users can
migrate without needing to infer the new structure.
--
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]
