ephraimbuddy commented on code in PR #65543:
URL: https://github.com/apache/airflow/pull/65543#discussion_r3116370142
##########
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 think keeping it as bool creates a clearer seam for overrides. With bool,
an override's contract is: log your own specifics, return False to skip. With
exceptions, overrides has to understand that the caller will catch exception
and log the db error "error initializing bundle" regardless of what really
failed e.g http errors from an API-backed override). Main goal is to be able to
swap the db calls with api backed calls. Also, resolve_callback_bundle already
returns None for the same reason rather than raising
--
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]
