Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

External session for _AsyncBase - #70

Merged
abdelhai merged 2 commits into
deta:nextfrom
jnsougata:master
May 19, 2022
Merged

External session for _AsyncBase#70
abdelhai merged 2 commits into
deta:nextfrom
jnsougata:master

Conversation

@jnsougata

Copy link
Copy Markdown

Previously it was just creating the client session internally. There's no option to use external session. Now as session is an optioal arg user may pass in an external session otherwise it will create it's own session automatically.

@abdelhai

Copy link
Copy Markdown
Contributor

hi @jnsougata could you explain (with use case code) how this behavior is better?

@abdelhai
abdelhai requested review from aavshr and abdelhai May 17, 2022 19:42
@abdelhai abdelhai added help wanted Extra attention is needed investigating labels May 17, 2022
@lemonyte lemonyte mentioned this pull request May 17, 2022
@jnsougata

jnsougata commented May 18, 2022

Copy link
Copy Markdown
Author

hi @jnsougata could you explain (with use case code) how this behavior is better?

Let's say I am making a discord bot using aiohttp. So I will already be having an aiohttp client session. If I want to integrate async base there, then I will like to use that client session rather than internally created client session. In this case if we let async base create another session internally that will increase the number of sessions which is no good.

For example:
The connect method actually assigns the session to be used for API calls. This is how I made my lib. I want it into this lib for more flexibility...

async def setup_hook(self) -> None:
        deta = Deta(os.getenv('DETA_TOKEN'))
        await deta.connect(session=self.http._HTTPClient__session, loop=self.loop)
        self.db = deta.base('01PIXEL')
        self.drive = deta.drive('PixeL_@11223344')

@abdelhai

Copy link
Copy Markdown
Contributor

@jnsougata, thank you for the additional useful input!
seems like we need to rethink the async-base spec. we will start working on it soon and use your PR as a reference.

@abdelhai
abdelhai changed the base branch from master to next May 19, 2022 09:36
@abdelhai

Copy link
Copy Markdown
Contributor

i'm merging this into the next branch which will be the base for the major async release. thanks!

@abdelhai
abdelhai merged commit b667893 into deta:next May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

help wanted Extra attention is needed investigating

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants