-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixing everything #123
fixing everything #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, nice. So, am I understanding correctly that you have moved the message sending code (that used to be in Rasa) to a celery task, so it's now fully async? The delay issue is no longer noticeable?
scheduler/celery_tasks.py
Outdated
@@ -395,11 +393,38 @@ def resume_and_trigger(self, # pylint: disable=unused-argument | |||
if response_resume.status_code != 200: | |||
raise Exception() | |||
|
|||
response_intent = send_trigger(user_id, trigger) | |||
if response_intent != 200: | |||
print('Exception, retrying') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we log this instead of printing it, or is that not appropriate here? Also I would make the message slightly more descriptive - maybe "Exception during resume_and_trigger" or something, to at least say which function it was happening it (same for the others).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely yes, I will do this.
scheduler/celery_tasks.py
Outdated
recipient_id = mes['recipient_id'] | ||
message = mes['text'] | ||
client.post_message(int(recipient_id), message) | ||
if ENVIRONMENT == 'prod': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no delays simulated in testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe I'd better keep it always active indeed.
@raar1 Exactly, and it runs so smoothly now that I could cry. |
Co-authored-by: Robin Richardson <raar1@users.noreply.github.com>
SonarCloud Quality Gate failed. 0 Bugs 0.0% Coverage Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Fixes PerfectFit-project/virtual-coach-issues#382