Add child class for single callable #46
Conversation
for more information, see https://pre-commit.ci
| class TQDMProgressSubscriber(TQDMPublisher): | ||
| def __init__(self, iterable, *, announcer: "?", request_id: str, message: dict, **tqdm_kwargs): | ||
| super().__init__(iterable, **tqdm_kwargs) | ||
|
|
||
| def on_progress_update(format_dict) -> None: | ||
| """ | ||
| Describe what this announcer is all about... | ||
| """ | ||
| announcer.announce(dict(request_id=request_id, **message)) | ||
|
|
||
| self.subscribe(callback=on_progress_update) |
There was a problem hiding this comment.
Oops, I meant to splinter this to a different PR/branch, my bad
What would you think about this structure?
There was a problem hiding this comment.
Main question is, what is the announcer class (is it something from Flask?)
There was a problem hiding this comment.
Yes, the announcer is from the GUIDE server—so that has to be used within a user-provided callable to the TQDMProgressSubscriber.
Unless you want to bring assumptions about the server-side event system implemented in the GUIDE (in contrast to the WebSockets used in our demos) to tqdm_publisher.
- MessageAnnouncer class declaration: https://github.com/NeurodataWithoutBorders/nwb-guide/blob/main/pyflask/sse.py
- Flask SSE handler: https://github.com/NeurodataWithoutBorders/nwb-guide/blob/4ab4e1f682d9bd9e1b4b3d79f9c0e2e593bddea9/pyflask/manageNeuroconv/manage_neuroconv.py#L688
There was a problem hiding this comment.
A fair amount of that listener logic seems quite general, so could very well be included along with this child class
What is the specific point at which it interacts with the GUIDE client?
There was a problem hiding this comment.
At the two locations defined above. The SSE handler waits for new messages to be generated by calling MessageAnnouncer.announce in other endpoints.
There was a problem hiding this comment.
Response in NeurodataWithoutBorders/nwb-guide#695 and #49.
These introduce a tqdm_publisher global import, though, since the listen_to_neuroconv_events endpoint needs to have a reference to a single MessageAnnouncer instance used across progress updates when the user requests that they'd like to receive updates from that endpoint.
If we can't tolerate that, then this line of inquiry is infeasible since whatever queue we use has to be available in several endpoints and the simplest way to use this queue (i.e. with the least assumptions, most flexibility) would be in a callback as the initial commit for this PR was already doing.
|
@garrettmflynn Can this be closed now I assume? |
Changed in alignment with NeurodataWithoutBorders/nwb-guide#676 (comment).
Publishing initially in Draft Mode for further review and discussion about our approach.