Skip to content

Conversation

@HsiuChuanHsu
Copy link
Contributor

@HsiuChuanHsu HsiuChuanHsu commented Aug 23, 2025

Description

When working on #54115, a good suggestion from @hterik that we should refactors the Kubernetes executor type to make for better readability.
This PR refactors the Kubernetes executor type system by converting simple tuple aliases to structured NamedTuple classes.

Changes Made

  • Type System Improvements

    • KubernetesResultsTypeKubernetesResults (NamedTuple)
    • KubernetesWatchTypeKubernetesWatch (NamedTuple)
    • KubernetesJobTypeKubernetesJob (NamedTuple)
  • Code Updates

    • Updated kubernetes_executor_types.py: Define new NamedTuple classes with proper typing
    • Updated kubernetes_executor_utils.py: Modify usage patterns to work with NamedTuples
    • Updated kubernetes_executor.py: Refactor result handling logic for new type structure
    • Updated test_kubernetes_executor.py: Adjust test assertions to work with named fields

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Aug 23, 2025
Copy link
Contributor

@hterik hterik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. 👍
LGTM

@HsiuChuanHsu HsiuChuanHsu force-pushed the refactor/kubernetes-types-to-namedtuple branch from 7af7305 to 07e01a5 Compare August 25, 2025 23:03
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@uranusjr
Copy link
Member

Need to fix CI.

Copy link
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for the PR!
LGTM if CI run is successful.

Unfortunately, linting still needs to be fixed in this run. It can be automatically resolved by using prek (the alternative to pre-commit we recently switched to) with the following command.

prek ruff --all-files && prek run ruff-format --all-files

@HsiuChuanHsu HsiuChuanHsu force-pushed the refactor/kubernetes-types-to-namedtuple branch from a81a4d7 to 27251fe Compare August 26, 2025 22:47
Replace complex tuple type aliases with NamedTuple classes for better
readability and maintainability:
- KubernetesResults: key, state, pod_name, namespace, resource_version, failure_details
- KubernetesWatch: pod_name, namespace, state, annotations, resource_version, failure_details
- KubernetesJob: key, command, kube_executor_config, pod_template_file
…adability

Replace tuple type aliases with structured NamedTuple classes in Kubernetes
executor to improve code maintainability and type safety.
…tesResults object

- Simplify _change_state method signature from 5 parameters to 1 KubernetesResults object
- Update method calls in sync() and _flush_result_queue() to pass KubernetesResults directly
- Update all test cases to create KubernetesResults objects instead of passing individual parameters
…ject

Update test_pod_failure_logging_with_container_terminated to create KubernetesResults object
Update test_pod_failure_logging_exception_handling to use new _change_state signature
Update test_pod_failure_logging_non_failed_state to use KubernetesResults object

- Remove unused FailureDetails import
- Format KubernetesResults constructor calls for better readability
@HsiuChuanHsu HsiuChuanHsu force-pushed the refactor/kubernetes-types-to-namedtuple branch from 27251fe to bcb1a51 Compare August 26, 2025 23:27
@HsiuChuanHsu
Copy link
Contributor Author

Finally pass the CI test! Thanks for all of review.

@eladkal eladkal requested a review from uranusjr August 27, 2025 04:57
@uranusjr uranusjr force-pushed the refactor/kubernetes-types-to-namedtuple branch from e53394e to bcb1a51 Compare August 27, 2025 08:58
@uranusjr uranusjr merged commit 700b43c into apache:main Aug 27, 2025
87 checks passed
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
nothingmin pushed a commit to nothingmin/airflow that referenced this pull request Sep 2, 2025
@HsiuChuanHsu HsiuChuanHsu deleted the refactor/kubernetes-types-to-namedtuple branch September 9, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants