o-nikolas commented on code in PR #65269:
URL: https://github.com/apache/airflow/pull/65269#discussion_r3120843132


##########
airflow-core/newsfragments/65269.significant.rst:
##########
@@ -0,0 +1 @@
+Synchronous deadline callbacks (``SyncCallback``) can now access Connections, 
Variables, XCom, and Assets from the Airflow metadata database.

Review Comment:
   Do we really think Assets are required in a callback? Personally I'd start 
with the minimal set because it's easier to add things later than remove them, 
and I can't really think of a (common) usecase for assets. The rest make sense 
to me.



##########
task-sdk/src/airflow/sdk/execution_time/callback_supervisor.py:
##########
@@ -52,6 +72,15 @@ class _BundleInfoLike(Protocol):
 log: FilteringBoundLogger = 
structlog.get_logger(logger_name="callback_supervisor")
 
 
+# The set of messages that a callback subprocess can send to the supervisor.
+# This is a minimal subset of ToSupervisor: read-only access to Connections,
+# Variables, and Assets, plus MaskSecret for the secrets masker.
+CallbackToSupervisor = Annotated[
+    GetAssetByName | GetAssetByUri | GetConnection | GetVariable | MaskSecret,

Review Comment:
   No XCOM? That's mentioned at the top in the doc, but not here? Or does it 
work differently?



##########
task-sdk/src/airflow/sdk/execution_time/callback_supervisor.py:
##########
@@ -52,6 +72,15 @@ class _BundleInfoLike(Protocol):
 log: FilteringBoundLogger = 
structlog.get_logger(logger_name="callback_supervisor")
 
 
+# The set of messages that a callback subprocess can send to the supervisor.
+# This is a minimal subset of ToSupervisor: read-only access to Connections,
+# Variables, and Assets, plus MaskSecret for the secrets masker.
+CallbackToSupervisor = Annotated[
+    GetAssetByName | GetAssetByUri | GetConnection | GetVariable | MaskSecret,

Review Comment:
   Mentioned above, but for what it's worth, I would swap Asset with XCom. I 
think the latter is much more useful to a callback. But I'd be happy with 
neither too, to start.



-- 
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]

Reply via email to