Lee-W commented on code in PR #65543:
URL: https://github.com/apache/airflow/pull/65543#discussion_r3115853029
##########
airflow-core/src/airflow/dag_processing/manager.py:
##########
@@ -569,14 +577,26 @@ def _fetch_callbacks(
guard.commit()
return callback_queue
- def _add_callback_to_queue(self, request: CallbackRequest):
- self.log.debug("Queuing %s CallbackRequest: %s",
type(request).__name__, request)
+ def resolve_callback_bundle(self, request: CallbackRequest) ->
BaseDagBundle | None:
+ """
+ Resolve the bundle required to execute a callback request.
+
+ Returns ``None`` if the bundle is no longer configured.
+ """
try:
- bundle = DagBundlesManager().get_bundle(name=request.bundle_name,
version=request.bundle_version)
+ return DagBundlesManager().get_bundle(name=request.bundle_name,
version=request.bundle_version)
except ValueError:
- # Bundle no longer configured
self.log.error("Bundle %s no longer configured, skipping
callback", request.bundle_name)
return None
+
+ def initialize_callback_bundle(self, request: CallbackRequest, bundle:
BaseDagBundle) -> bool:
Review Comment:
I feel we should use error handling instead of returning a `bool` (i.e.,
raise an exception and handle it in _add_callback_to_queue)
##########
airflow-core/src/airflow/dag_processing/manager.py:
##########
@@ -586,7 +606,16 @@ def _add_callback_to_queue(self, request: CallbackRequest):
request.bundle_name,
request.bundle_version,
)
- return None
+ return False
+ return True
+
+ def _add_callback_to_queue(self, request: CallbackRequest):
+ self.log.debug("Queuing %s CallbackRequest: %s",
type(request).__name__, request)
+ bundle = self.resolve_callback_bundle(request)
+ if bundle is None:
+ return None
+ if not self.initialize_callback_bundle(request, bundle):
+ return None
Review Comment:
```suggestion
def _add_callback_to_queue(self, request: CallbackRequest) -> None:
self.log.debug("Queuing %s CallbackRequest: %s",
type(request).__name__, request)
bundle = self.resolve_callback_bundle(request)
if bundle is None:
return
try:
self.initialize_callback_bundle(request, bundle):
except Exception:
return
```
I guess we won't be able to narrow the exception here?
--
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]
