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

add async methods to the sync client class #22

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

tddschn
Copy link
Contributor

@tddschn tddschn commented May 5, 2023

console.print(await self.data)


class ModelzClient:
Copy link
Member

Choose a reason for hiding this comment

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

Why there are two ModelzClient? Can they be the same one or inherit from some base class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing my code. I could rename modelz/aioclient.py's ModelzClient to something else, like AioModelzClient, would that work?

Yes they could inherit from the same ABC, but since the classes are simple, I don't see much benefit doing that now.


async def _post(self, url, content, timeout):
headers = self.auth.get_headers()
async with aiohttp.ClientSession() as session:
Copy link
Member

Choose a reason for hiding this comment

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

Reuse the session.

Copy link
Contributor Author

@tddschn tddschn May 6, 2023

Choose a reason for hiding this comment

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

I tried that, see https://github.com/tddschn/nssurge-api/blob/f2aafe1b01877b8ff8bfc6e74a58736a7ba3b058/nssurge_api/api.py#L30

Definding self.session in init() would require also defining __aenter__ and __aexit__ methods, and request methods that uses self.session to use an async context manager (see https://github.com/tddschn/nssurge-cli/blob/112e600fad6299d4f78012b7aaa20dfb0cdb9758/nssurge_cli/cap_commands.py#L20), due to how aiohttp works.

I'll try to figure out a better way to support session reuse.

modelz/aioclient.py Outdated Show resolved Hide resolved
modelz/aioclient.py Show resolved Hide resolved
modelz/aioclient.py Show resolved Hide resolved
Comment on lines +42 to +53
async def save_to_file(self, file: str):
with open(file, "wb") as f:
f.write(await self.data)

@property
async def data(self) -> Any:
if not self._data:
self._data = self.serde.decode(await self.resp.content.read())
return self._data

async def show(self):
console.print(await self.data)
Copy link
Member

Choose a reason for hiding this comment

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

Does the sync interface work with this?

modelz/env.py Show resolved Hide resolved
modelz/utils.py Show resolved Hide resolved
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.

2 participants