pierrejeambrun commented on code in PR #55862:
URL: https://github.com/apache/airflow/pull/55862#discussion_r2368968909
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -83,8 +85,34 @@ class DAGRunResponse(BaseModel):
note: str | None
dag_versions: list[DagVersionResponse]
bundle_version: str | None
+ bundle_name: str | None = Field(validation_alias=AliasPath("dag_model",
"bundle_name"), exclude=True)
dag_display_name: str = Field(validation_alias=AliasPath("dag_model",
"dag_display_name"))
+ # Mypy issue https://github.com/python/mypy/issues/1362
+ @computed_field # type: ignore[prop-decorator]
+ @property
+ def bundle_url(self) -> str | None:
+ if not self.bundle_name:
+ 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)
+ # fallback to the deprecated option if the bundle model does not
have a signed_url_template
+ # attribute
+ try:
+ return DagBundlesManager().view_url(self.bundle_name,
self.bundle_version)
+ except ValueError:
+ return None
Review Comment:
Nit: We can probably make a mixin out of this, or a base class, so both
`DAGRunResponse` and `DagVersionResponse` can re-use it. That would probably
make it easier to maintain, and remove the risk of de-syncing them.
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/dag_run.py:
##########
@@ -83,8 +85,34 @@ class DAGRunResponse(BaseModel):
note: str | None
dag_versions: list[DagVersionResponse]
bundle_version: str | None
+ bundle_name: str | None = Field(validation_alias=AliasPath("dag_model",
"bundle_name"), exclude=True)
dag_display_name: str = Field(validation_alias=AliasPath("dag_model",
"dag_display_name"))
+ # Mypy issue https://github.com/python/mypy/issues/1362
+ @computed_field # type: ignore[prop-decorator]
+ @property
+ def bundle_url(self) -> str | None:
+ if not self.bundle_name:
+ 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)
+ # fallback to the deprecated option if the bundle model does not
have a signed_url_template
+ # attribute
+ try:
+ return DagBundlesManager().view_url(self.bundle_name,
self.bundle_version)
+ except ValueError:
+ return None
Review Comment:
This was taken out of `DagVersionResponse`. You are right this design is
not good, but I would say that this improvement would be for a follow up PR.
Also to mention that in `DagVersionResponse` this wasn't done like that
originally, and a db access was added later on when doing `Refactor bundle
view_url to not instaniate bundle on server components` which I guess needed db
access.
--
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]
