Skip to content

Commit

Permalink
Fix yoda-conditions (apache#44466)
Browse files Browse the repository at this point in the history
Yoda conditions revert expdcted / checked values. We are adding
ruff rule to avoid them.
  • Loading branch information
potiuk authored Nov 29, 2024
1 parent fdd353a commit 0334901
Show file tree
Hide file tree
Showing 232 changed files with 2,803 additions and 2,433 deletions.
2 changes: 1 addition & 1 deletion dev/breeze/src/airflow_breeze/utils/packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ def format_version_suffix(version_suffix: str) -> str:
"""
if version_suffix:
if "." == version_suffix[0] or "+" == version_suffix[0]:
if version_suffix[0] == "." or version_suffix[0] == "+":
return version_suffix
else:
return f".{version_suffix}"
Expand Down
10 changes: 5 additions & 5 deletions dev/breeze/src/airflow_breeze/utils/selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1326,10 +1326,10 @@ def is_amd_runner(self) -> bool:
"""
return any(
[
"amd" == label.lower()
or "amd64" == label.lower()
or "x64" == label.lower()
or "asf-runner" == label
label.lower() == "amd"
or label.lower() == "amd64"
or label.lower() == "x64"
or label == "asf-runner"
or ("ubuntu" in label and "arm" not in label.lower())
for label in json.loads(self.runs_on_as_json_public)
]
Expand All @@ -1346,7 +1346,7 @@ def is_arm_runner(self) -> bool:
"""
return any(
[
"arm" == label.lower() or "arm64" == label.lower() or "asf-arm" == label
label.lower() == "arm" or label.lower() == "arm64" or label == "asf-arm"
for label in json.loads(self.runs_on_as_json_public)
]
)
Expand Down
6 changes: 3 additions & 3 deletions dev/breeze/tests/test_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,17 @@ def test_get_provider_requirements():

def test_get_removed_providers():
# Modify it every time we schedule provider for removal or remove it
assert [] == get_removed_provider_ids()
assert get_removed_provider_ids() == []


def test_get_suspended_provider_ids():
# Modify it every time we suspend/resume provider
assert [] == get_suspended_provider_ids()
assert get_suspended_provider_ids() == []


def test_get_suspended_provider_folders():
# Modify it every time we suspend/resume provider
assert [] == get_suspended_provider_folders()
assert get_suspended_provider_folders() == []


@pytest.mark.parametrize(
Expand Down
4 changes: 2 additions & 2 deletions docker_tests/test_prod_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ def test_without_command(self, default_docker_image):
"""Checking the image without a command. It should return non-zero exit code."""
with pytest.raises(DockerException) as ctx:
run_cmd_in_docker(image=default_docker_image)
assert 2 == ctx.value.return_code
assert ctx.value.return_code == 2

def test_airflow_command(self, default_docker_image):
"""Checking 'airflow' command. It should return non-zero exit code."""
with pytest.raises(DockerException) as ctx:
run_airflow_cmd_in_docker(image=default_docker_image)
assert 2 == ctx.value.return_code
assert ctx.value.return_code == 2

def test_airflow_version(self, default_docker_image):
"""Checking 'airflow version' command. It should return zero exit code."""
Expand Down
33 changes: 18 additions & 15 deletions helm_tests/airflow_aux/test_airflow_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_dags_mount(self, dag_values, expected_mount):
],
)

assert 3 == len(docs)
assert len(docs) == 3
for doc in docs:
assert expected_mount in jmespath.search("spec.template.spec.containers[0].volumeMounts", doc)

Expand Down Expand Up @@ -164,7 +164,7 @@ def test_annotations(self):
],
)

assert 7 == len(k8s_objects)
assert len(k8s_objects) == 7

for k8s_object in k8s_objects:
if k8s_object["kind"] == "CronJob":
Expand Down Expand Up @@ -225,25 +225,28 @@ def test_global_affinity_tolerations_topology_spread_constraints_and_node_select
],
)

assert 12 == len(k8s_objects)
assert len(k8s_objects) == 12

for k8s_object in k8s_objects:
if k8s_object["kind"] == "CronJob":
podSpec = jmespath.search("spec.jobTemplate.spec.template.spec", k8s_object)
else:
podSpec = jmespath.search("spec.template.spec", k8s_object)

assert "foo" == jmespath.search(
"affinity.nodeAffinity."
"requiredDuringSchedulingIgnoredDuringExecution."
"nodeSelectorTerms[0]."
"matchExpressions[0]."
"key",
podSpec,
assert (
jmespath.search(
"affinity.nodeAffinity."
"requiredDuringSchedulingIgnoredDuringExecution."
"nodeSelectorTerms[0]."
"matchExpressions[0]."
"key",
podSpec,
)
== "foo"
)
assert "user-node" == jmespath.search("nodeSelector.type", podSpec)
assert "static-pods" == jmespath.search("tolerations[0].key", podSpec)
assert "foo" == jmespath.search("topologySpreadConstraints[0].topologyKey", podSpec)
assert jmespath.search("nodeSelector.type", podSpec) == "user-node"
assert jmespath.search("tolerations[0].key", podSpec) == "static-pods"
assert jmespath.search("topologySpreadConstraints[0].topologyKey", podSpec) == "foo"

@pytest.mark.parametrize(
"expected_image,tag,digest",
Expand Down Expand Up @@ -384,7 +387,7 @@ def test_have_all_config_mounts_on_init_containers(self):
"templates/dag-processor/dag-processor-deployment.yaml",
],
)
assert 5 == len(docs)
assert len(docs) == 5
expected_mount = {
"subPath": "airflow.cfg",
"name": "config",
Expand Down Expand Up @@ -424,7 +427,7 @@ def test_priority_class_name(self):
],
)

assert 10 == len(docs)
assert len(docs) == 10
for doc in docs:
component = doc["metadata"]["labels"]["component"]
if component == "airflow-cleanup-pods":
Expand Down
40 changes: 21 additions & 19 deletions helm_tests/airflow_aux/test_basic_helm_chart.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ def test_basic_deployments(self, version):
if chart_name and "postgresql" in chart_name:
continue
k8s_name = k8s_object["kind"] + ":" + k8s_object["metadata"]["name"]
assert "TEST-VALUE" == labels.get(
"test-label"
assert (
labels.get("test-label") == "TEST-VALUE"
), f"Missing label test-label on {k8s_name}. Current labels: {labels}"

def test_basic_deployments_with_standard_naming(self):
Expand Down Expand Up @@ -254,8 +254,8 @@ def test_basic_deployment_with_standalone_dag_processor(self, version):
if chart_name and "postgresql" in chart_name:
continue
k8s_name = k8s_object["kind"] + ":" + k8s_object["metadata"]["name"]
assert "TEST-VALUE" == labels.get(
"test-label"
assert (
labels.get("test-label") == "TEST-VALUE"
), f"Missing label test-label on {k8s_name}. Current labels: {labels}"

@pytest.mark.parametrize("version", ["2.3.2", "2.4.0", "default"])
Expand Down Expand Up @@ -493,7 +493,7 @@ def test_annotations_on_airflow_pods_in_deployment(self):
)
# pod_template_file is tested separately as it has extra setup steps

assert 8 == len(k8s_objects)
assert len(k8s_objects) == 8

for k8s_object in k8s_objects:
annotations = k8s_object["spec"]["template"]["metadata"]["annotations"]
Expand Down Expand Up @@ -600,8 +600,8 @@ def test_postgres_connection_url_no_override(self):
show_only=["templates/secrets/metadata-connection-secret.yaml"],
)[0]
assert (
"postgresql://postgres:[email protected]:5432/postgres?sslmode=disable"
== base64.b64decode(doc["data"]["connection"]).decode("utf-8")
base64.b64decode(doc["data"]["connection"]).decode("utf-8")
== "postgresql://postgres:[email protected]:5432/postgres?sslmode=disable"
)

def test_postgres_connection_url_pgbouncer(self):
Expand All @@ -612,9 +612,9 @@ def test_postgres_connection_url_pgbouncer(self):
values={"pgbouncer": {"enabled": True}},
)[0]
assert (
"postgresql://postgres:[email protected]:6543/"
base64.b64decode(doc["data"]["connection"]).decode("utf-8")
== "postgresql://postgres:[email protected]:6543/"
"my-release-metadata?sslmode=disable"
== base64.b64decode(doc["data"]["connection"]).decode("utf-8")
)

def test_postgres_connection_url_pgbouncer_use_standard_naming(self):
Expand All @@ -625,9 +625,9 @@ def test_postgres_connection_url_pgbouncer_use_standard_naming(self):
values={"useStandardNaming": True, "pgbouncer": {"enabled": True}},
)[0]
assert (
"postgresql://postgres:[email protected]:6543/"
base64.b64decode(doc["data"]["connection"]).decode("utf-8")
== "postgresql://postgres:[email protected]:6543/"
"my-release-metadata?sslmode=disable"
== base64.b64decode(doc["data"]["connection"]).decode("utf-8")
)

def test_postgres_connection_url_name_override(self):
Expand All @@ -639,8 +639,8 @@ def test_postgres_connection_url_name_override(self):
)[0]

assert (
"postgresql://postgres:postgres@overrideName:5432/postgres?sslmode=disable"
== base64.b64decode(doc["data"]["connection"]).decode("utf-8")
base64.b64decode(doc["data"]["connection"]).decode("utf-8")
== "postgresql://postgres:postgres@overrideName:5432/postgres?sslmode=disable"
)

def test_priority_classes(self):
Expand Down Expand Up @@ -685,9 +685,10 @@ def test_redis_broker_connection_url(self):
show_only=["templates/secrets/redis-secrets.yaml"],
values={"redis": {"enabled": True, "password": "test1234"}},
)[1]
assert "redis://:test1234@my-release-redis:6379/0" == base64.b64decode(
doc["data"]["connection"]
).decode("utf-8")
assert (
base64.b64decode(doc["data"]["connection"]).decode("utf-8")
== "redis://:test1234@my-release-redis:6379/0"
)

def test_redis_broker_connection_url_use_standard_naming(self):
# no nameoverride, redis and useStandardNaming
Expand All @@ -696,9 +697,10 @@ def test_redis_broker_connection_url_use_standard_naming(self):
show_only=["templates/secrets/redis-secrets.yaml"],
values={"useStandardNaming": True, "redis": {"enabled": True, "password": "test1234"}},
)[1]
assert "redis://:test1234@my-release-airflow-redis:6379/0" == base64.b64decode(
doc["data"]["connection"]
).decode("utf-8")
assert (
base64.b64decode(doc["data"]["connection"]).decode("utf-8")
== "redis://:test1234@my-release-airflow-redis:6379/0"
)

@staticmethod
def default_trigger_obj(version):
Expand Down
8 changes: 4 additions & 4 deletions helm_tests/airflow_aux/test_celery_kubernetes_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def test_should_create_a_worker_deployment_with_the_celery_executor(self):
show_only=["templates/workers/worker-deployment.yaml"],
)

assert "config" == jmespath.search("spec.template.spec.volumes[0].name", docs[0])
assert "dags" == jmespath.search("spec.template.spec.volumes[1].name", docs[0])
assert jmespath.search("spec.template.spec.volumes[0].name", docs[0]) == "config"
assert jmespath.search("spec.template.spec.volumes[1].name", docs[0]) == "dags"

def test_should_create_a_worker_deployment_with_the_celery_kubernetes_executor(self):
docs = render_chart(
Expand All @@ -45,5 +45,5 @@ def test_should_create_a_worker_deployment_with_the_celery_kubernetes_executor(s
show_only=["templates/workers/worker-deployment.yaml"],
)

assert "config" == jmespath.search("spec.template.spec.volumes[0].name", docs[0])
assert "dags" == jmespath.search("spec.template.spec.volumes[1].name", docs[0])
assert jmespath.search("spec.template.spec.volumes[0].name", docs[0]) == "config"
assert jmespath.search("spec.template.spec.volumes[1].name", docs[0]) == "dags"
Loading

0 comments on commit 0334901

Please sign in to comment.