adding post-abort start and active check endpoints #200
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
evolver/device.py
Outdated
| @@ -71,6 +71,10 @@ def schema(self): | |||
| "controllers": [{"kind": str(type(a)), "config": a.Config.model_json_schema()} for a in self.controllers], | |||
| } | |||
|
|
|||
| @property | |||
| def active(self): | |||
| return self.enable_control and self.enable_commit | |||
There was a problem hiding this comment.
I'm just wondering how this might intuitively get interpreted by someone manually controlling the hardware, i.e., not using control but committing commands?
"active" sounds to me like they should be able to but the above implies that you shouldn't/can't. We could have a "running" status flag, however at that point, you literally just have these two flags. Perhaps it's best to nuke this state reduction and just natively report both enable_control and enable_commit in the healthz resp?
There was a problem hiding this comment.
Really lack of commit is what should make it inactive, but no control would generally mean nothing to commit.
But also I wonder if maybe these needn't be separate. Perhaps I can just make enable_control mean both execute control and commit - the separation is probably not too useful...
There was a problem hiding this comment.
Really lack of commit is what should make it inactive, but no control would generally mean nothing to commit.
Exactly.
But also I wonder if maybe these needn't be separate. Perhaps I can just make enable_control mean both execute control and commit - the separation is probably not too useful...
Do you mean separate at all or just as reported from healthz?
With the way we have the configuration setup, I def think it's needed to have both. One deactivates hardware control (enable_commit) and the other starts the controller(s)/experiment running (enable_control). Removing enable_control would require someone who wants to pause an experiment (control) and manually interact with the hardware to have to stop the running experiment by creating a new config with the control section omitted. Seems burdensome to me, no?
There was a problem hiding this comment.
Really lack of commit is what should make it inactive, but no control would generally mean nothing to >commit.
Exactly.
Actually, I'm not sure if that's entirely accurate as enable_commit is only checked in evolver.device.Evolver.loop_once and not at any communication layer. So a user could programmatically control the hardware still from the SDK (but not the app).
|
I've gotten rid of Also just FYI, if there is control done via web API calls, this would operate independent of loop anyhow. Abort relates just to activity of the in-process loop (in addition of course to stopping devices). If that ought to change, we can do so separate from this PR |
Resolves #198
Does so by adding a start endpoint which is basically just an update setting
enable_controlandenable_committo true. It also adds an "active" flag, exposed both in healthz and state, which indicates if those activities are being done.