amoghrajesh commented on code in PR #52876:
URL: https://github.com/apache/airflow/pull/52876#discussion_r2197821691
##########
airflow-core/src/airflow/dag_processing/bundles/manager.py:
##########
@@ -124,12 +174,44 @@ def parse_config(self) -> None:
@provide_session
def sync_bundles_to_db(self, *, session: Session = NEW_SESSION) -> None:
self.log.debug("Syncing DAG bundles to the database")
+
+ def _extract_and_sign_template(bundle_name: str) -> tuple[str | None,
dict]:
+ bundle_instance = self.get_bundle(name)
+ new_template_ = bundle_instance.view_url_template()
+ new_params_ = self._extract_template_params(bundle_instance)
+ if new_template_:
+ if not _is_safe_bundle_url(new_template_):
+ self.log.warning(
+ "Bundle %s has unsafe URL template '%s', skipping URL
update",
+ bundle_name,
+ new_template_,
+ )
+ new_template_ = None
+ else:
+ # Sign the URL for integrity verification
+ new_template_ = _sign_bundle_url(new_template_,
bundle_name)
+ self.log.debug("Signed URL template for bundle %s",
bundle_name)
+ return new_template_, new_params_
Review Comment:
Really nice!
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_versions.py:
##########
@@ -41,10 +42,21 @@ class DagVersionResponse(BaseModel):
@property
def bundle_url(self) -> str | None:
if self.bundle_name:
- try:
- return DagBundlesManager().view_url(self.bundle_name,
self.bundle_version)
- except ValueError:
- return None
+ # Get the bundle model from the database and render the URL
+ from airflow.models.dagbundle import DagBundleModel
+ from airflow.utils.session import create_session
+
+ with create_session() as session:
+ bundle_model = session.scalar(
+ select(DagBundleModel).where(DagBundleModel.name ==
self.bundle_name)
+ )
+
+ if bundle_model and hasattr(bundle_model,
"signed_url_template"):
+ return bundle_model.render_url(self.bundle_version)
+ try:
+ return DagBundlesManager().view_url(self.bundle_name,
self.bundle_version)
Review Comment:
Can we add a comment here please?
##########
airflow-core/src/airflow/models/dagbundle.py:
##########
@@ -32,6 +34,8 @@ class DagBundleModel(Base):
- active: Is the bundle currently found in configuration?
- version: The latest version Airflow has seen for the bundle.
- last_refreshed: When the bundle was last refreshed.
+ - signed_url_template: Signed URL template for viewing the bundle
+ - template_params: JSON object containing template parameters (e.g.,
{"subdir": "dags"})
Review Comment:
```suggestion
- template_params: JSON object containing template parameters for
constructing view url (e.g., {"subdir": "dags"})
```
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dags.py:
##########
@@ -507,8 +507,10 @@ class TestDagDetails(TestDagEndpoint):
],
)
@pytest.mark.usefixtures("configure_git_connection_for_dag_bundle")
+ @mock.patch("airflow.api_fastapi.core_api.datamodels.dag_versions.hasattr")
Review Comment:
Non blocking, but create a fixture here?
(skip if you think it doesn't make sense)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
