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

PluginUUIDBase's force_new_uuid option seems broken #15

Closed
wapiflapi opened this issue Jul 23, 2020 · 5 comments · Fixed by #16
Closed

PluginUUIDBase's force_new_uuid option seems broken #15

wapiflapi opened this issue Jul 23, 2020 · 5 comments · Fixed by #16
Assignees
Labels
bug Something isn't working

Comments

@wapiflapi
Copy link

When using PluginUUIDBase's force_new_uuid option and setting it to True I'm always getting the same uuid. Which is the opposite of what I expect.

It looks like when self.value is None a new uuid is generated, and this works fine when for the first request. But subsequent request seem to be using the same id.

I suspect the code that needs to be fixed is missing a self.value = None before trying anything else here:

@wapiflapi
Copy link
Author

Thinking about this, why is self.value being used anyway? I don't understand why this is being stored on the plugin instance. It seems like a normal variable would be enough and match the intend better, and it would have avoided this bug.

@tomwojcik
Copy link
Owner

Hey, thanks for opening this ticket. Indeed that's a bug. I'll fix this within a few days.

@tomwojcik tomwojcik self-assigned this Jul 25, 2020
@tomwojcik tomwojcik added the bug Something isn't working label Jul 25, 2020
@tomwojcik tomwojcik linked a pull request Jul 26, 2020 that will close this issue
@tomwojcik
Copy link
Owner

0.2.3 is published. Please try again and let me know if it's all good now.

@wapiflapi
Copy link
Author

Looks good from here!

Thank you for the quick fix.

FTR my workaround was:

# Fix https://github.com/tomwojcik/starlette-context/issues/15

__original_extract_value_from_header_by_key = (
    PluginUUIDBase.extract_value_from_header_by_key)


async def __patched_extract_value_from_header_by_key(self, request):
    self.value = None
    return await __original_extract_value_from_header_by_key(self, request)


PluginUUIDBase.extract_value_from_header_by_key = (
    __patched_extract_value_from_header_by_key
)

Removing this from my code, and using 0.2.3 all my tests still pass.
🎆 For me it looks good!

@tomwojcik
Copy link
Owner

Awesome! Thanks for checking that out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants