Adding typing info for HTEX #2847
Conversation
…TEX.connected_managers
| def outstanding(self): | ||
| def outstanding(self) -> int: | ||
| """Returns the count of tasks outstanding across the interchange | ||
| and managers""" | ||
| outstanding_c = self.command_client.run("OUTSTANDING_C") | ||
| return outstanding_c |
There was a problem hiding this comment.
Suggestion: "while here," and having just added the typing, there's no longer a need for the interim variable:
- outstanding_c = self.command_client.run("OUTSTANDING_C")
- return outstanding_c
+ return self.command_client.run("OUTSTANDING_C")Same mild suggestion in the other hunks.
| """Returns a list of dicts one for each connected managers. | ||
| The dict contains info on manager(str:manager_id), block_id, | ||
| worker_count, tasks(int), idle_durations(float), active(bool) | ||
| """ |
There was a problem hiding this comment.
This structure is frustratingly close to ManagerRecord in parsl/executors/high_throughput/manager_record, but not quite - ManagerRecords are transformed here:
This docstring could be made into a very similar looking TypedDict, though, I think, which could give stronger checking of accesses to the dictionary
There was a problem hiding this comment.
Yep, this is annoyingly bad. There are two fields that are different: tasks, which is now an int and idle_duration which is computed from ManagerRecord.idle_since. I can update the docstring and try update the ManagerRecord in a separate PR.
There was a problem hiding this comment.
probably needs to be a different type, not ManagerRecord
the current ManagerRecord is only used inside the interchange, I think - so it could be renamed and/or moved into interchange.py
Description
This PR only adds some typing info for these methods/properties on the
HighThroughputExecutor:HTEX.outstandingHTEX.connected_workersHTEX.connected_managersType of change
Choose which options apply, and delete the ones which do not apply.