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

Use create_task when executing the 'after' handler coroutine #27

Merged
merged 2 commits into from
Sep 25, 2019
Merged

Use create_task when executing the 'after' handler coroutine #27

merged 2 commits into from
Sep 25, 2019

Conversation

proelke
Copy link
Collaborator

@proelke proelke commented Sep 24, 2019 •
edited
Loading

If a call is used in the 'after' handler, it can block the event loop, so it is better to run the coroutine in a task.

@proelke proelke added the bug Something isn't working label Sep 24, 2019
@proelke proelke self-assigned this Sep 24, 2019
@ghost
Copy link

ghost commented Sep 24, 2019 •
edited by ghost
Loading

DeepCode Report (#870093)

DeepCode analyzed this pull request.
There are no new issues.

tropxy
tropxy reviewed Sep 24, 2019
View reviewed changes
ocpp/v16/charge_point.py Outdated Show resolved Hide resolved
tropxy
tropxy suggested changes Sep 24, 2019
View reviewed changes
Copy link
Collaborator

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

Check it out boy

@proelke
Copy link
Collaborator Author

proelke commented Sep 24, 2019

Updated it to use ensure_future. @tropxy

@tropxy
Copy link
Collaborator

tropxy commented Sep 24, 2019

Did you test this? Just to be sure :P

@proelke
Copy link
Collaborator Author

proelke commented Sep 24, 2019

I did in-fact test it with @OrangeTux.

tropxy
tropxy approved these changes Sep 24, 2019
View reviewed changes
Copy link
Collaborator

@tropxy tropxy left a comment

Choose a reason for hiding this comment

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

Cool! Nice job ;)

@proelke proelke merged commit 873b2eb into master Sep 25, 2019
@proelke proelke deleted the after_handler_can_block branch September 25, 2019 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doumelas doumelas doumelas left review comments

@tropxy tropxy tropxy approved these changes

@OrangeTux OrangeTux Awaiting requested review from OrangeTux OrangeTux is a code owner

Assignees

@proelke proelke

Labels
bug Something isn't working
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@proelke @tropxy @doumelas