Fix databricks job link #45334
Fix databricks job link #45334
Conversation
|
|
||
| # Example logic: Extract the job run ID from the path | ||
| job_details = parsed.path.split("/")[-1].replace(".json", "") | ||
| databricks_workspace_url = f"https://<your-databricks-workspace-url>/#job/{job_details}/run" |
There was a problem hiding this comment.
how does this <your-databricks-workspace-url> in the URL work?
Also, can you provide an example of how the XCOM value looks like?
| @@ -2343,3 +2344,95 @@ def test_user_databricks_task_key(self): | |||
| expected_task_key = "test_task_key" | |||
|
|
|||
| assert expected_task_key == operator.databricks_task_key | |||
|
|
|||
|
|
|||
| def test_get_link_with_valid_s3_url(): | |||
There was a problem hiding this comment.
this test is doing a lot of mocking and does not seem to test much.
| value = XCom.get_value(key=XCOM_RUN_PAGE_URL_KEY, ti_key=ti_key) | ||
|
|
||
| # Check if the value is an S3 URL | ||
| if value and value.startswith("s3://"): |
There was a problem hiding this comment.
What if the implementation doesn't store an s3 url but just object key? What about other schemes that are not s3?
| @@ -242,7 +243,35 @@ def get_link( | |||
| *, | |||
| ti_key: TaskInstanceKey, | |||
| ) -> str: | |||
| return XCom.get_value(key=XCOM_RUN_PAGE_URL_KEY, ti_key=ti_key) | |||
| # Retrieve the value from XCom | |||
There was a problem hiding this comment.
I generally appreciate having comments, yet I think the method name get_value is already saying that a value is being retrieved
| parsed = urlparse(s3_url) | ||
|
|
||
| # Example logic: Extract the job run ID from the path | ||
| job_details = parsed.path.split("/")[-1].replace(".json", "") |
There was a problem hiding this comment.
Are we assuming that the json file path contains the job details? That's not true, the details (the complete URL) is stored inside the file
|
|
||
| # Example logic: Extract the job run ID from the path | ||
| job_details = parsed.path.split("/")[-1].replace(".json", "") | ||
| databricks_workspace_url = f"https://<your-databricks-workspace-url>/#job/{job_details}/run" |
There was a problem hiding this comment.
Is this clickable? How/where is <your-databricks-workspace-url> going to be replaced?
- 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.
potiuk
force-pushed
the
fix-databricks-job-link
branch
from
ccbca05 to
6e3fc9c
Compare
|
I rebased it @mohamedmeqlad99 -> we found and issue with @jscheffl with the new caching scheme - fixed in #45347 that would run "main" version of the tests. |
Fix Databricks Extra Link to See Job Run with Custom S3 Backend
Problem Addressed
Closes: #45240
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:
Proposed Changes
DatabricksJobRunLinkimplementation to reliably fetch the Databricks job run URL from XCom usingXCom.get_value.Key Benefits
Testing
Issue Link
Closes #45240.
Checklist
Code of Conduct
I agree to follow this project's Code of Conduct.