Tighten up typing of addresses in HTEX#4087
Conversation
Cleans up a couple instances of "stringly-typed" variables in the HTEX internal code, which previously passed the list of candidate addresses as a comma-separated string. While that can't be avoided in between `executor.py` and `process_worker_pool.py` (since the latter is started up as a shell command), it *can* be avoided within the executor class and in the `get_all_addresses` function, freeing us of one call each of `str.split` and `str.join`.
|
|
||
| if self.address: | ||
| self.all_addresses = address | ||
| self.all_addresses = {self.address} |
There was a problem hiding this comment.
is this a behaviour change that means users could previous supply multiple addresses in the htex executor init field separated by "," and now they cannot?
(I don't think this is really a problem because it's documented as a single address and there isn't a strong usecase for a user to supply multiple addresses - just trying to get my head around expected changes)
There was a problem hiding this comment.
Yeah, that would be a behavior change. I'm on the same page that this is an undocumented feature and should be safe to remove.
That said, should I add anything like a changelog entry or a dedicated test in this PR to handle that change more explicitly/robustly?
There was a problem hiding this comment.
no need for further docs or tests for that, i think
2 participants
Description
Cleans up a couple instances of "stringly-typed" variables in the HTEX internal code, which previously passed the list of candidate addresses as a comma-separated string. While that can't be avoided in between
executor.pyandprocess_worker_pool.py(since the latter is started up as a shell command), it can be avoided within the executor class and in theget_all_addressesfunction, freeing us of one call each ofstr.splitandstr.join.Changed Behaviour
None
Type of change