Skip to content

fix: prevent aiohttp from becoming mandatory to use the sync SDK - #441

Merged
ppicom merged 1 commit into
mainfrom
eg-4748-async-bootstrap
Sep 29, 2026
Merged

ppicom merged 1 commit into
mainfrom
eg-4748-async-bootstrap

Conversation

@ppicom

@ppicom ppicom commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

While testing the SDK I've found that to do from UnleashClient import UnleashClient now you have to have aiohttp installed. By splitting this files, now it can remain an optional dependency, like it was intended from the beginning.


Stack created with GitHub Stacks CLI • Give Feedback 💬

to do so, I've moved the async metrics reporter to its own file.

@daveleek daveleek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK good stuff!
Is there a reason the tests couldn't stay in the same file? The moving of the test code contributed to the most of this PRs lines of change, and made it a little more difficult to figure out which parts were critical to review and not. I think if would've been possible (maybe for a similar next time) I would've appreciated them being split in a separate PR

@ppicom
ppicom merged commit c3059bd into main Sep 29, 2026
9 checks passed
@ppicom
ppicom deleted the eg-4748-async-bootstrap branch September 29, 2026 12:17
@ppicom

ppicom commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

OK good stuff! Is there a reason the tests couldn't stay in the same file? The moving of the test code contributed to the most of this PRs lines of change, and made it a little more difficult to figure out which parts were critical to review and not. I think if would've been possible (maybe for a similar next time) I would've appreciated them being split in a separate PR

Ooff, totally. I just wanted to do it in one fell swoop. But it's true that the tests part made it hard. Noted for the next one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants