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.

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

send_attempt is an int in msisdn too #462

Merged
merged 6 commits into from Nov 10, 2021
Merged

send_attempt is an int in msisdn too #462

merged 6 commits into from Nov 10, 2021

Conversation

DMRobertson
Copy link
Contributor

@DMRobertson DMRobertson commented Nov 9, 2021

Like #460 and #461, but for phone numbers.

@DMRobertson DMRobertson requested a review from a team as a code owner November 9, 2021 14:46
@DMRobertson DMRobertson removed request for a team and clokep November 9, 2021 14:46
@DMRobertson
Copy link
Contributor Author

Oh, I need to rebase onto main and also add a changelog.

@DMRobertson DMRobertson requested review from clokep and a team November 9, 2021 14:50
@clokep
Copy link
Contributor

clokep commented Nov 9, 2021

Code looks fine, although linting is failing!

@DMRobertson
Copy link
Contributor Author

Code looks fine, although linting is failing!

Yeah. I think something going wrong with mocking an async function on 3.6 (works fine locally on 3.9). I'll investigate.

@babolivier
Copy link
Contributor

babolivier commented Nov 9, 2021

Yeah. I think something going wrong with mocking an async function on 3.6 (works fine locally on 3.9). I'll investigate.

Yeah awaiting on a Mock doesn't work on 3.6. In Synapse, we have make_awaitable to cover that: https://github.com/matrix-org/synapse/blob/bfb4b858a999684ba2459ee4c3aa20270d13062d/tests/test_utils/__init__.py#L49-L57 (so if your mock's return_value is "foo" you replace it with make_awaitable("foo").

Not sure why it's only an issue with sydent now, but I'd say that's probably the way to go?

@DMRobertson
Copy link
Contributor Author

Yeah awaiting on a Mock doesn't work on 3.6. In Synapse, we have make_awaitable to cover that:

Thanks, that does the trick.

Not sure why it's only an issue with sydent now, but I'd say that's probably the way to go?

Probably because we've never tried to mock an async function in Sydent before.

@DMRobertson DMRobertson merged commit 8381dcf into main Nov 10, 2021
@DMRobertson DMRobertson deleted the dmr/sendAttempt-int branch November 10, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants