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

Python 3.6 support through contextvars backport? #3

Closed
evinism opened this issue Nov 8, 2021 · 5 comments
Closed

Python 3.6 support through contextvars backport? #3

evinism opened this issue Nov 8, 2021 · 5 comments

Comments

@evinism
Copy link

evinism commented Nov 8, 2021

This project looks fantastic, thanks for creating it.

There's a relatively functional backport of contextvars to 3.6 -- I'm wondering whether or not adding it as a dependency would give this project instant python 3.6 support, or whether there's other Python 3.7 features it relies on such that that wouldn't work.

Backport here

@sondrelg
Copy link
Member

sondrelg commented Nov 8, 2021

Can't think of anything that would block usage other than the package dependency spec, but I'll investigate a little to double check 😊

@evinism
Copy link
Author

evinism commented Nov 8, 2021

ah, looking more closely, i'm wondering if MagicStack/contextvars#2 might make it impossible, since it lacks "asyncio support", whatever exactly that entails.

And it looks like they probably won't improve the backport to officially support it, sadly.

@sondrelg
Copy link
Member

sondrelg commented Nov 8, 2021

Looks like tests are passing in #4 for version 3.6 - I'm just having a little bit of trouble with the 3.10 run. Looks like I just need to tweak the dependencies a bit, but shouldn't be an issue I think.

If you want to test the branch on your project to see if it works, you should be able to install the middleware using pip install git+https://github.com/snok/asgi-correlation-id.git@229a5f3b5c44f7e228be166587ed1efe88b75cfe.

If it does work for you I'd be happy to add 3.6 support since it doesn't actually impact other versions at all 🙂

@evinism
Copy link
Author

evinism commented Nov 8, 2021

Yep, seems to work perfectly for my use case -- correlation ID gets added to the logs just as expected. Thanks a ton!

@sondrelg
Copy link
Member

sondrelg commented Nov 8, 2021

Sweet! I'll push a release as soon as I can get the dependency spec to do what I want 😄

Thanks for the suggestion 👏

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

No branches or pull requests

2 participants