PLAT-10588: Implemented concurrent DF loop #158
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
March 22, 2021 14:12
symphony-elias
requested review from
thibauult,
symphony-youri,
symphony-mariacristina and
symphony-youness
symphony-elias
commented
Mar 23, 2021
| for event in filter(lambda e: e is not None, events): | ||
| for listener in self.listeners: | ||
| if await listener.is_accepting_event(event, self.bdk_config.bot.username): | ||
| await self._dispatch_on_event_type(listener, event) | ||
| asyncio.create_task(self._dispatch_on_event_type(listener, event)) |
There was a problem hiding this comment.
do we have to do something in term of error handling? I remember that is the legacy BDK there were done callbacks used
if an exception is raised in the task it won't break the DF loop or anything?
There was a problem hiding this comment.
to support the soft/hard kill will we have to collect the tasks somewhere so we can cancel() all of the running ones/
There was a problem hiding this comment.
If an exception is raised inside the listener, then asyncio logs it for us:
2021-03-24 10:01:58,850 - root - DEBUG - iIaUeR - Received event: Ojwe2s5mMfJbHPlLbEZRUH___oech-ZubQ
2021-03-24 10:02:03,208 - asyncio - ERROR - - Task exception was never retrieved
future: <Task finished name='Task-13' coro=<AbstractDatafeedLoop._dispatch_on_event_type() done, defined at /Users/elias.croze/workspace/IdeaProjects/symphony-api-client-python/symphony/bdk/core/service/datafeed/abstract_datafeed_loop.py:150> exception=ValueError('error inside listener')>
Traceback (most recent call last):
File "/Users/elias.croze/workspace/IdeaProjects/symphony-api-client-python/symphony/bdk/core/service/datafeed/abstract_datafeed_loop.py", line 163, in _dispatch_on_event_type
await listener_method(event.initiator, event_field)
File "/Users/elias.croze/workspace/IdeaProjects/symphony-api-client-python/examples/datafeed.py", line 28, in on_message_sent
raise ValueError("error inside listener")
ValueError: error inside listener
IMHO this is enough.
There was a problem hiding this comment.
To retrieve unfinished tasks, we can use: https://docs.python.org/3/library/asyncio-task.html#asyncio.all_tasks
There was a problem hiding this comment.
is there a risk we will retrieve user's define tasks and stop them (like if the bot is doing something else beside running the DF loop)?
There was a problem hiding this comment.
I added a list with the listener tasks. Please bear in mind lists are thread safe: https://stackoverflow.com/a/6319267
symphony-youri
approved these changes
Mar 29, 2021
symphony-elias
added a commit
that referenced
this pull request
Apr 6, 2021
* PLAT-10433: Sphinx documentation (#130) * PLAT-10483 - HealthService implementation (#132) - HealthService implementation and tests - Renamed test classes from `test_name_of_class.py` to `name_of_class_test.py` * PLAT-10532: ApplicationService implementation (#133) * ApplicationService implementation * Update documentation * PLAT-10530: Presence service implementation (#134) - Presence service implementation and tests - Put the presence service test payloads in one JSON file - Added type-hint to the ServiceFactory.get_health_service function * PLAT-10530 Fixed documentation issues in links regarding the presence service (#135) This fix was added to correctly show links to the endpoint documentation when sphinx documentation is generated. * PLAT-10531 Signal service implementation (#138) * PLAT-10529 Ability to set private key and certificate content programmatically (#137) Private key and Certificate were able to only be loaded from the config file when initializing bdk. In this PR, we update bot config to be able to set private key and certificate after bdk being initialized. * PLAT-10489: Implemented automatic pagination (#139) * PLAT-10643: Load certs from system store In #122 we enable loading a custom cert store for the HTTP client. This broked using pods with valid certs because the system certs are no longer loaded. This change loads them even all the time (and we might add custom ones on top of that). Also sync the rest.mustache template used for code generation. * PLAT-10643: Unit test system certs loading * Added forgotten default value in list_all_stream_members (#144) * PLAT-10533: Added User-Agent header in requests (#141) * PLAT-10600 enforce pylint checks in PR builder (#146) Goal of this PR is to enable pylint checks in the PR builder. For now the limit to fail is set to 9.50 (max being 10.00). We can start like this and if needed we can adapt it later on, if we think that is too strict. All changes about fixture are related to this issue: https://stackoverflow.com/questions/46089480/pytest-fixtures-redefining-name-from-outer-scope-pylint * PLAT-10534: Added X-Trace-Id header to each HTTP call (#147) * PLAT-10534: Added X-Trace-Id header to each HTTP call * Replaced single quotes with double quotes * Fixed python version in GH workflow * Updated caching logic in workflows (#148) * Changed caching logic of poetry deps to be specific to the actual python version. * Updated build, push and pylint to cache pip dependencies as well * PLAT-10651: Updated generated code following role type update to string (#149) * Updated generated code using openApiGen 5.0.1 and updated role format to str * Updated unit tests and examples * PLAT-10565 Adding utils classes (#150) Adding some util classes on message processing to: Extract entities from a given incoming message (mentions, hashtags, cashtags, emojis) Extract message content from presentationML Pre-process an outgoing message by "cleaning" the text message, escaping all special characters that will violate messageML format Convert a stream id into a url safe id and viceversa * PLAT-10535: Extension App RSA authentication (#153) * PLAT-10535: First implementation of RSA ext app authentication * Added and improved docstrings * Fixed some pylint errors * Updated poetry deps * #143 Activity API (#151) * #143 Activity API (draft version) * #143 Added unit tests * #143 Added markdown doc * #143 Documented AbstractActivity class * PLAT-10709: Switch from python-jose to PyJWT (#159) Switch from using python-jose to using PyJWT * PLAT-10588: Implemented concurrent DF loop (#158) * PLAT-10588: Implemented concurrency of event handling in DF loop * Refactored datafeed loops * Updated poetry deps * PLAT-10563: FormReply activity (#161) - Base classes for a FormReply Activity. - Modification of the command activity to parse attributes in the constructor instead - Fixed related tests * PLAT-10538: Minor improvements on extension app authenticator (#163) * PLAT-10538: added allowed audience = app-id when validating jwt * Improved error handling in ExtensionAppAuthenticatorRsa.validate_jwt * Updated dependencies * Bumped version to 2.0b1 * PR builder should build 2.0-rc branch Co-authored-by: symphony-youness <76746033+symphony-youness@users.noreply.github.com> Co-authored-by: symphony-hong <65538951+symphony-hong@users.noreply.github.com> Co-authored-by: Mariacristina De Dominicis <65179248+symphony-mariacristina@users.noreply.github.com> Co-authored-by: Soufiane Aourinmouche <52406574+symphony-soufiane@users.noreply.github.com> Co-authored-by: Youri Bonnaffe <youri.bonnaffe@symphony.com> Co-authored-by: Youri Bonnaffé <63661676+symphony-youri@users.noreply.github.com> Co-authored-by: Thibault Pensec <39826516+symphony-thibault@users.noreply.github.com>
symphony-youness
added a commit
to symphony-youness/symphony-api-client-python
that referenced
this pull request
Jun 4, 2021
* PLAT-10433: Sphinx documentation (finos#130) * PLAT-10483 - HealthService implementation (finos#132) - HealthService implementation and tests - Renamed test classes from `test_name_of_class.py` to `name_of_class_test.py` * PLAT-10532: ApplicationService implementation (finos#133) * ApplicationService implementation * Update documentation * PLAT-10530: Presence service implementation (finos#134) - Presence service implementation and tests - Put the presence service test payloads in one JSON file - Added type-hint to the ServiceFactory.get_health_service function * PLAT-10530 Fixed documentation issues in links regarding the presence service (finos#135) This fix was added to correctly show links to the endpoint documentation when sphinx documentation is generated. * PLAT-10531 Signal service implementation (finos#138) * PLAT-10529 Ability to set private key and certificate content programmatically (finos#137) Private key and Certificate were able to only be loaded from the config file when initializing bdk. In this PR, we update bot config to be able to set private key and certificate after bdk being initialized. * PLAT-10489: Implemented automatic pagination (finos#139) * PLAT-10643: Load certs from system store In finos#122 we enable loading a custom cert store for the HTTP client. This broked using pods with valid certs because the system certs are no longer loaded. This change loads them even all the time (and we might add custom ones on top of that). Also sync the rest.mustache template used for code generation. * PLAT-10643: Unit test system certs loading * Added forgotten default value in list_all_stream_members (finos#144) * PLAT-10533: Added User-Agent header in requests (finos#141) * PLAT-10600 enforce pylint checks in PR builder (finos#146) Goal of this PR is to enable pylint checks in the PR builder. For now the limit to fail is set to 9.50 (max being 10.00). We can start like this and if needed we can adapt it later on, if we think that is too strict. All changes about fixture are related to this issue: https://stackoverflow.com/questions/46089480/pytest-fixtures-redefining-name-from-outer-scope-pylint * PLAT-10534: Added X-Trace-Id header to each HTTP call (finos#147) * PLAT-10534: Added X-Trace-Id header to each HTTP call * Replaced single quotes with double quotes * Fixed python version in GH workflow * Updated caching logic in workflows (finos#148) * Changed caching logic of poetry deps to be specific to the actual python version. * Updated build, push and pylint to cache pip dependencies as well * PLAT-10651: Updated generated code following role type update to string (finos#149) * Updated generated code using openApiGen 5.0.1 and updated role format to str * Updated unit tests and examples * PLAT-10565 Adding utils classes (finos#150) Adding some util classes on message processing to: Extract entities from a given incoming message (mentions, hashtags, cashtags, emojis) Extract message content from presentationML Pre-process an outgoing message by "cleaning" the text message, escaping all special characters that will violate messageML format Convert a stream id into a url safe id and viceversa * PLAT-10535: Extension App RSA authentication (finos#153) * PLAT-10535: First implementation of RSA ext app authentication * Added and improved docstrings * Fixed some pylint errors * Updated poetry deps * finos#143 Activity API (finos#151) * finos#143 Activity API (draft version) * finos#143 Added unit tests * finos#143 Added markdown doc * finos#143 Documented AbstractActivity class * PLAT-10709: Switch from python-jose to PyJWT (finos#159) Switch from using python-jose to using PyJWT * PLAT-10588: Implemented concurrent DF loop (finos#158) * PLAT-10588: Implemented concurrency of event handling in DF loop * Refactored datafeed loops * Updated poetry deps * PLAT-10563: FormReply activity (finos#161) - Base classes for a FormReply Activity. - Modification of the command activity to parse attributes in the constructor instead - Fixed related tests * PLAT-10538: Minor improvements on extension app authenticator (finos#163) * PLAT-10538: added allowed audience = app-id when validating jwt * Improved error handling in ExtensionAppAuthenticatorRsa.validate_jwt * Updated dependencies * Bumped version to 2.0b1 * PR builder should build 2.0-rc branch Co-authored-by: symphony-youness <76746033+symphony-youness@users.noreply.github.com> Co-authored-by: symphony-hong <65538951+symphony-hong@users.noreply.github.com> Co-authored-by: Mariacristina De Dominicis <65179248+symphony-mariacristina@users.noreply.github.com> Co-authored-by: Soufiane Aourinmouche <52406574+symphony-soufiane@users.noreply.github.com> Co-authored-by: Youri Bonnaffe <youri.bonnaffe@symphony.com> Co-authored-by: Youri Bonnaffé <63661676+symphony-youri@users.noreply.github.com> Co-authored-by: Thibault Pensec <39826516+symphony-thibault@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket
PLAT-10588
Description
Implemented concurrency of event handlings in the datafeed loop.
Refactored the DF loops along with unit tests.
Checklist