Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Correctly await sendSMS #413

Merged
merged 3 commits into from
Oct 8, 2021
Merged

Correctly await sendSMS #413

merged 3 commits into from
Oct 8, 2021

Conversation

babolivier
Copy link
Contributor

@babolivier babolivier commented Oct 8, 2021 •
edited
Loading

Fixing this warning, which might be the cause of #411

Oct  8 14:16:14 corus sydent-vectoris[21655]: /opt/vectoris/sydent/sydent/validators/msisdnvalidator.py:131: RuntimeWarning: coroutine 'OpenMarketSMS.sendTextSMS' was never awaited
Oct  8 14:16:14 corus sydent-vectoris[21655]:   self.omSms.sendTextSMS(smsBody, msisdn, originator)
Oct  8 14:16:14 corus sydent-vectoris[21655]: RuntimeWarning: Enable tracemalloc to get the object allocation traceback

@babolivier babolivier requested a review from a team October 8, 2021 15:26
@DMRobertson DMRobertson self-assigned this Oct 8, 2021
DMRobertson
DMRobertson reviewed Oct 8, 2021
View reviewed changes
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable. I wonder if mypy or similar could have spotted this; doesn't look like it's setup for sydent.

changelog.d/413.bugfix Outdated Show resolved Hide resolved
@DMRobertson DMRobertson removed their assignment Oct 8, 2021
@babolivier babolivier merged commit 8fb54d5 into release-v2.4 Oct 8, 2021
@babolivier babolivier deleted the babolivier/await_sendsms branch October 8, 2021 15:34
@DMRobertson DMRobertson mentioned this pull request Oct 8, 2021
Closed
@babolivier babolivier mentioned this pull request Oct 8, 2021
Closed
@babolivier
Copy link
Contributor Author

babolivier commented Oct 8, 2021 •
edited
Loading

Likely broke when converting to async/await in v2.4.0. We believe running the function in a fire-and-forget worked with inline callbacks and would run the function in the background, but would just not run it when using the async/await syntax.

@DMRobertson
Copy link
Contributor

Related: #310 ?

@clokep clokep mentioned this pull request Oct 14, 2021
Closed
@DMRobertson DMRobertson mentioned this pull request Oct 26, 2021
Closed
@DMRobertson
Copy link
Contributor

Seems reasonable. I wonder if mypy or similar could have spotted this; doesn't look like it's setup for sydent.

Looks like mypy wouldn't have caught this at present, at least not without a plugin. See python/mypy#2499.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@squahtx squahtx squahtx left review comments

@DMRobertson DMRobertson DMRobertson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@babolivier @DMRobertson @squahtx