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

Fix spark operator log retrieval from driver #38106

Merged
merged 3 commits into from
Mar 31, 2024

Conversation

sudohainguyen
Copy link
Contributor

@sudohainguyen sudohainguyen commented Mar 13, 2024

Related logs:
image

closes: #37681

Overriding default BASE_CONTAINER_NAME assigns the container_logs attribute correctly in further steps, so that in execution airflow can get logs from the correct container.


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

@sudohainguyen
Copy link
Contributor Author

@eladkal mind taking a look? think this minor change can fix

Copy link
Contributor

@dirrao dirrao left a comment

Choose a reason for hiding this comment

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

LGTM. Can you add the test case for this?

@sudohainguyen
Copy link
Contributor Author

LGTM. Can you add the test case for this?

sure

@sudohainguyen sudohainguyen force-pushed the fix/issue-37681-cncf branch 9 times, most recently from 7e94ea5 to 2a47d16 Compare March 13, 2024 18:43
@sudohainguyen
Copy link
Contributor Author

@dirrao tests added!

@sudohainguyen sudohainguyen requested a review from dirrao March 14, 2024 16:18
@sudohainguyen sudohainguyen force-pushed the fix/issue-37681-cncf branch 7 times, most recently from 39bf49d to 79bb051 Compare March 19, 2024 01:40
@hamedhsn
Copy link
Contributor

hamedhsn commented Mar 19, 2024

LGTM that makes sense since the older version of k8soperator uses that base container name and not supported multiple container

Signed-off-by: Hai Nguyen <quanghai.ng1512@gmail.com>
Signed-off-by: Hai Nguyen <quanghai.ng1512@gmail.com>
@sudohainguyen sudohainguyen force-pushed the fix/issue-37681-cncf branch 2 times, most recently from 42d4b9e to 11e0681 Compare March 31, 2024 05:34
Signed-off-by: Harry <quanghai.ng1512@gmail.com>
Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

LGTM as a bug fix, but it would be nice to support overriding the Spark job container name (in a separate PR).

@hussein-awala hussein-awala merged commit ec6091d into apache:main Mar 31, 2024
49 checks passed
@sudohainguyen sudohainguyen deleted the fix/issue-37681-cncf branch March 31, 2024 12:45
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.

SparkKubernetesOperator not retrieves logs from the driver pod and displays them in the Airflow UI.
5 participants