Messenger Interface rework #357
Conversation
…v/dialog_flow_framework into feat/graceful_termination
# Conflicts: # chatsky/messengers/common/interface.py # chatsky/pipeline/pipeline/pipeline.py # chatsky/pipeline/types.py # poetry.lock # tests/pipeline/test_messenger_interface.py
There was a problem hiding this comment.
Could you also do a self-review of these changes?
I enabled CI on this PR, make sure it passes.
Didn't review the last two test cases -- need more doc to understand how the BaseTestPollingInterface should be used and how the changes made to its method assist in testing.
I think the task of inheriting tg interface from this one should be done here instead of #365, so I'll close it. I think it's better to test this rework on a real interface before merging.
Also, I think it is worth considering moving #332 in this PR as well. Multiple messenger interfaces feature would be very convenient for testing this implementation.
| def not_obtained_updates(self): | ||
| if len(self.received_updates) == len(self.expected_updates): | ||
| self.obtained_updates = True | ||
| return not self.obtained_updates |
There was a problem hiding this comment.
Why not assert self.expected_updates == self.recieved_updates?
Add docs to make purpose of the method clear.
| self.requests = [] | ||
| self.expected_updates = [] |
There was a problem hiding this comment.
These should be passed in __init__.
| while self.running: | ||
| await asyncio.sleep(0.05) |
There was a problem hiding this comment.
Add comments on the purpose of these changes to the methods (what they do for the test).
Co-authored-by: Roman Zlobin <[email protected]>
Co-authored-by: Roman Zlobin <[email protected]>
…v/dialog_flow_framework into feat/graceful_termination
…n, many minor review changes
…v/dialog_flow_framework into feat/graceful_termination
Description
PollingMessengerInterfaceclass. Now requests are put in a queue, then done by 'workers', so that requests from several context_id's can be processed asynchronously. And a few other reasons that are explained in theTriggersPR, I think.MessengerInterface. When SIGINT is received, instead of abruptly terminating,shutdown()fromMessengerInterfaceis called, gracefully shutting down the program. All requests received prior to that are processed normally, then the program shuts down.MessengerInterfacebase class, so thatconnect()is run as an async task, not blocking the code.Checklist
To Consider
connect()methods for different Interface classes require different arguments, which is handled by a bunch of "if" statements inrun_in_foreground()at theMessengerInterfaceclass. There has to be a better way.