amoghrajesh commented on code in PR #52876:
URL: https://github.com/apache/airflow/pull/52876#discussion_r2194711769
##########
airflow-core/src/airflow/dag_processing/bundles/manager.py:
##########
@@ -124,12 +174,45 @@ 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 _signed_template(new_template_: str | None, bundle_name: str) ->
str | None:
+ 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_
+
stored = {b.name: b for b in session.query(DagBundleModel).all()}
+
for name in self._bundle_config.keys():
+ # Update URL template and parameters if they've changed
+ bundle_instance = self.get_bundle(name)
+ new_template = bundle_instance.view_url_template()
+ new_params = self._extract_template_params(bundle_instance)
Review Comment:
I wonder if we can just move extraction of these two directly into the
signed_template and call it:
`extract_and_sign_bundle_template(bundle_instance)` or even better move line
197 too. That way, it only takes bundle_name.
##########
airflow-core/src/airflow/dag_processing/bundles/base.py:
##########
@@ -218,6 +218,9 @@ def remove_stale_bundle_versions(self):
with other processes.
"""
log.info("checking for stale bundle versions locally")
+
+ from airflow.dag_processing.bundles.manager import DagBundlesManager
+
Review Comment:
nit: import above log?
##########
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)
+ except ValueError:
+ return None
Review Comment:
Now that I think of this, try catch is probably not needed because the
`view_url` would handle that.
--
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]
