Skip to content

Conversation

@mohamed-mis
Copy link
Contributor

This PR addresses an issue in the CustomObjectLauncher operator within the Kubernetes provider, where submitting a SparkApplication manifest -using the SparkKubernetesOperator- results in the loss of existing metadata labels and annotations.

The current implementation in get_body() overwrites the entire metadata object:
self.body.metadata = {"name": self.name, "namespace": self.namespace}
This discards any pre-existing metadata such as labels, annotations, or custom fields.
This PR replaces the overwrite with an in-place update using dict.update().

Testing:
This fix has been validated with the following setup:

  • Airflow Version: 3.0.6
  • Deployment Method: Official Helm chart 1.18.0
  • Kubernetes Provider Version: apache-airflow-providers-cncf-kubernetes==10.7.0
  • Python Version: 3.12.11

SparkApplication resources submitted via the SparkKubernetesOperator now retain their original metadata labels as expected.


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

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 24, 2025

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@boring-cyborg boring-cyborg bot added area:providers provider:cncf-kubernetes Kubernetes (k8s) provider related issues labels Sep 24, 2025
@mohamed-mis
Copy link
Contributor Author

mohamed-mis commented Sep 26, 2025

All GHA checks were successful, except for [CI image checks / Static checks], which failed due missing licence text in other files not related to the PR.

Copy link
Contributor

@nevcohen nevcohen left a comment

Choose a reason for hiding this comment

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

Looks good! Just add test

@mohamed-mis
Copy link
Contributor Author

tests for the custom_object_launcher already exist and covers the updated part.
test_custom_object_launcher.py

@eladkal
Copy link
Contributor

eladkal commented Sep 29, 2025

tests for the custom_object_launcher already exist and covers the updated part.

Your PR changes/fix behavior thus you can't claim tests already cover the fix.
You don't have to write a new test, it could be that extend or fix to a current test is enough. The idea is that the change you are implemented here won't be regressed in future versions so the way to make sure it doesn't happen is with test coverage.

test suite for get_body logic in CustomObjectLauncher, including metadata handling
@mohamed-mis
Copy link
Contributor Author

Tests added:

  • test_get_body_initializes_metadata_when_missing
  • test_get_body_replaces_non_dict_metadata
  • test_get_body_preserves_existing_metadata_labels

Waiting for GHA to finish all workflows, otherwise it should be good (fingers Crossed).

@nevcohen
Copy link
Contributor

Now it's ready. It looks great!

@eladkal eladkal merged commit 422c6eb into apache:main Sep 30, 2025
90 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 30, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 1, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 2, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 3, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 4, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 5, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 7, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 8, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 9, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 10, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 11, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 12, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
dabla pushed a commit to dabla/airflow that referenced this pull request Oct 12, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 14, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 15, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 17, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
abdulrahman305 bot pushed a commit to abdulrahman305/airflow that referenced this pull request Oct 19, 2025
…plication manifest (apache#56063)

* Preserve existing metadata labels in SparkApplication manifest

Ensure that CustomObjectLauncher updates only the 'name' and 'namespace' fields in the metadata without overwriting existing labels or annotations.
This change uses dict.update() to safely modify the metadata in place, preventing loss of important SparkApplication metadata during submission.

* misc: format committed lines for consistency

* Fix: Safely handle missing `metadata` attribute in SparkJobSpec

* Add unit tests for get_body method in CustomObjectLauncher

test suite for get_body logic in CustomObjectLauncher, including metadata handling
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.

3 participants