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(databricks): update DatabricksJobRunLink for improved XCom handlin #45328

Closed

Conversation

mohamedmeqlad99
Copy link

Fix Databricks Extra Link to See Job Run with Custom S3 Backend

Problem Addressed

This PR addresses the issue described in #45240, where the Databricks Extra Link for "See Job Run" does not work when using a custom XCom backend that writes to S3. Specifically:

  • The link directs to the S3 path of the XCom JSON file instead of the Databricks workspace job run URL.
  • This behavior makes the feature unusable in setups with custom XCom backends, such as those storing XCom values entirely in S3.

Proposed Changes

  • Refactored the DatabricksJobRunLink implementation to reliably fetch the Databricks job run URL from XCom using XCom.get_value.
  • Ensured compatibility with custom XCom backends, decoupling the link generation logic from specific backend implementations.
  • Improved error handling to ensure the system fails gracefully if the required XCom value is missing or malformed.
  • Enhanced maintainability with detailed comments and a cleaner structure.

Key Benefits

  • Resolves the issue of broken links when using custom XCom backends.
  • Ensures compatibility across a wide range of backend configurations, including setups where all XCom values are stored in S3.
  • Improves user experience by providing accurate links to Databricks job run details.

Testing

  • Validated the new implementation using a local Airflow environment with a custom XCom backend.
  • Confirmed that the generated links now correctly point to Databricks job run URLs, regardless of the XCom storage location.

Issue Link

Closes #45240.

Checklist

  • All unit tests pass.
  • Code is formatted according to Airflow’s standards.
  • Changes are backward compatible and do not break existing functionality.

Code of Conduct

I agree to follow this project's Code of Conduct.

- Refactored the DatabricksJobRunLink class to include a more robust implementation for retrieving the job run URL from XCom.
- Updated to utilize XCom.get_value for improved clarity and maintainability.
- Enhanced error handling to prevent issues when the XCom key does not exist.
- Added detailed comments to clarify logic and improve readability.

This change ensures better alignment with Airflow's standards for operator links and enhances the reliability of job run URL retrieval.
Copy link

boring-cyborg bot commented Jan 1, 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 pre-commits 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: [email protected]
    Slack: https://s.apache.org/airflow-slack

@potiuk
Copy link
Member

potiuk commented Jan 1, 2025

You need to add unit tests exhibiting and testing the new behaviour

@mohamedmeqlad99
Copy link
Author

@potiuk i don't understand can u explain more how can I do this and what I need to edit

@potiuk
Copy link
Member

potiuk commented Jan 1, 2025

Add unit tests: Look at our Contributing docs - particularly testing - you can also look at other PRs for reference. We practically never accept PRs that have no accompanying unit tests and you need to learn how to write and run them if you want to contribute.

Our contribution guides have quick-starts for various IDEs and when you follow them you can have working development environment where you can run unit tests in less than 10 minutes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Databricks Extra Links See Job Run does not work with custom S3 backend
2 participants