Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace pod_manager.read_pod_logs with client.read_namespaced_pod_log in KubernetesPodOperator._write_logs #39112

Merged
merged 7 commits into from
May 5, 2024

Conversation

Lee-W
Copy link
Member

@Lee-W Lee-W commented Apr 18, 2024

Bring back the changes in #36228 and improve the type annotation.

As mentioned in #36228, we don't really need to call pod_manager.read_pod_logs and could call client.read_namespaced_pod_log directly instead


^ 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 newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes provider related issues labels Apr 18, 2024
@Lee-W Lee-W force-pushed the simplify-kpo-write-logs-logic branch from 6552b38 to df0b6fe Compare April 18, 2024 15:38
@Lee-W
Copy link
Member Author

Lee-W commented Apr 22, 2024

I'll hold this for some time. I will prepare it for review when I finish all the implementations.

@Lee-W Lee-W force-pushed the simplify-kpo-write-logs-logic branch from df0b6fe to 07826ef Compare April 30, 2024 10:29
@Lee-W Lee-W changed the title Simplify kpo write logs logic Replace pod_manager.read_pod_logs with client.read_namespaced_pod_log in KubernetesPodOperator._write_logs Apr 30, 2024
@Lee-W Lee-W marked this pull request as ready for review April 30, 2024 10:35
@eladkal eladkal requested a review from romsharon98 May 2, 2024 03:50
@Lee-W Lee-W force-pushed the simplify-kpo-write-logs-logic branch from 07826ef to 8c8179e Compare May 3, 2024 06:50
@Lee-W Lee-W force-pushed the simplify-kpo-write-logs-logic branch from 8c8179e to 6a2c750 Compare May 3, 2024 08:16
@eladkal eladkal merged commit 0e6c0ab into apache:main May 5, 2024
49 checks passed
RodrigoGanancia pushed a commit to RodrigoGanancia/airflow that referenced this pull request May 10, 2024
… in KubernetesPodOperator._write_logs (apache#39112)

* style(cncf): add type annotation and remove redundant else-block

* refactor(cncf): move deprecated method to the end for better readability

* style(cncf): use f-string instead of string concatenation

* refactor(cncf): reduce one comparison for should_delete_pod

* refactor(cncf): rename write_logs as _write_logs

* refactor(cncf): move read_pod_logs logic to _write_logs

* test(cncf): unify mock.patch as patch
@Lee-W Lee-W deleted the simplify-kpo-write-logs-logic branch June 5, 2024 06: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 provider related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants