Skip to content

Conversation

@dstandish
Copy link
Contributor

@dstandish dstandish commented Dec 22, 2022

This means we don't have to use ti.hostname as a proxy for pod name, and allows us to lift the 63 charcter limit, which was a consequence of getting pod name through hostname.

Using @uranusjr-style "protected" methods just, as always, to contain "public" surface area

@boring-cyborg boring-cyborg bot added provider:cncf-kubernetes Kubernetes (k8s) provider related issues area:logging labels Dec 22, 2022
@dstandish dstandish requested a review from ashb December 22, 2022 22:59
@dstandish dstandish force-pushed the allow-long-k8s-exec-pod-names branch from 1b0a294 to 374b042 Compare December 22, 2022 23:02
Copy link
Member

Choose a reason for hiding this comment

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

Instead of raising an error (which I think will result in a 500!) should we do something like we do on L191-192:

                log += f"*** {str(e)}\n"
                return log, {"end_of_log": True}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is already in a try / except so that catches and does just that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so i could duplicate that code or just raise :)

@dstandish dstandish force-pushed the allow-long-k8s-exec-pod-names branch from 4bdcd38 to ad89b4e Compare December 26, 2022 08:04
This means we don't have to use ti.hostname as a proxy for pod name, and allows us to lift the 63 charcter limit, which was a consequence of getting pod name through hostname.
@dstandish dstandish force-pushed the allow-long-k8s-exec-pod-names branch from ad89b4e to d61fe03 Compare December 27, 2022 20:29
@dstandish dstandish added this to the Airflow 2.6.0 milestone Dec 29, 2022
@dstandish dstandish merged commit c22fc00 into apache:main Dec 30, 2022
@dstandish dstandish deleted the allow-long-k8s-exec-pod-names branch December 30, 2022 23:11
@pierrejeambrun pierrejeambrun added the type:improvement Changelog: Improvements label Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:logging provider:cncf-kubernetes Kubernetes (k8s) provider related issues type:improvement Changelog: Improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants