jscheffl commented on code in PR #43435:
URL: https://github.com/apache/airflow/pull/43435#discussion_r1819482807
##########
providers/src/airflow/providers/edge/models/edge_worker.py:
##########
@@ -144,6 +144,12 @@ class EdgeWorker(BaseModel, LoggingMixin):
sysinfo: str
model_config = ConfigDict(from_attributes=True,
arbitrary_types_allowed=True)
+ class SetStateReturn(BaseModel):
Review Comment:
This probably will not work as the serialization converts the object on the
API to a String if not known. See line 320
(https://github.com/apache/airflow/pull/43435/files#diff-92ac90b9bbc292b8c1936377c9c5767eacffd72c422239b027e306a78622d35dR320)
of the same file how to register new classes for serialization mapping on the
interface.
But in general the change now changes to a custom object as response in
comparison to `list[str]` + Exception before. Do we need to change the
communication from the server or could we just change the way how the
`EdgeWorkerVersionException` is handled during the heartbeat and raise the
exception just during heartbeat but let other calls pass on the server side so
that a graceful job completion is possible (today all calls are blocked if
version mis-match)
##########
providers/src/airflow/providers/edge/models/edge_worker.py:
##########
@@ -144,6 +144,12 @@ class EdgeWorker(BaseModel, LoggingMixin):
sysinfo: str
model_config = ConfigDict(from_attributes=True,
arbitrary_types_allowed=True)
+ class SetStateReturn(BaseModel):
+ """Defines return type of set_state function."""
+
+ queues: Optional[List[str]] = None # noqa: UP006,UP007 - prevent
Sphinx failing
Review Comment:
Why is Sphinx failing here? Is it the same like in line 137?
Could we make this a top-level class and not a nested class (except if
object is not needed according to commend above)?
--
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]
