-
Notifications
You must be signed in to change notification settings - Fork 114
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
Fix blocking calls inside the event loop #383
Conversation
Have you tested this? |
Of course I did. Are you running into problems? |
I have now also added a fix for the other blocking calls |
@@ -227,10 +227,17 @@ async def fetch(self, data_type, end_date=None, areas=None): | |||
|
|||
res = await asyncio.gather(*jobs) | |||
|
|||
raw = [self._parse_json(i, areas) for i in res] | |||
raw = [await self._async_parse_json(i, areas) for i in res] |
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.
It would be more efficient to run the whole list comp in the executor instead of creating a job for each item being iterated
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.
True! And good catch. Honestly I didn't put to much thought in it as it was my first priority to create a quick fix. Actually, a lot that's happening in the integration should be done in a library, but that is a lot more work for which I don't have time atm
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.
It wasnt supported in the library. Im the one that created PR for the async interface in nordpool package. At the time it wasnt merged and i didnt want to wait. There were issues like using mutable default arguments.
No description provided.