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

fix for provider overriding in litestar #120

Merged

Conversation

lesnik512
Copy link
Member

No description provided.

@lesnik512 lesnik512 linked an issue Nov 17, 2024 that may be closed by this pull request
@lesnik512 lesnik512 force-pushed the 119-bug-provider-overriding-does-not-works-with-litestar branch from d70db4e to b79c9e8 Compare November 18, 2024 17:09
Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
tests/integrations/test_litestar_di.py 100.00% <100.00%> (ø)
that_depends/providers/base.py 100.00% <100.00%> (ø)

🚨 Try these New Features:

@lesnik512 lesnik512 changed the title add failing test fix for provider overriding in litestar Nov 18, 2024
@lesnik512
Copy link
Member Author

@nightblure It can be fixed by forbidding deepcopy of providers. Seems like litestar doing deepcopy of router or sth like that

@lesnik512
Copy link
Member Author

lesnik512 commented Nov 18, 2024

@vrslev @alexanderlazarev0 Hi! What do you think? Is it safe to redefine deepcopy method to forbid providers cloning?

P.S. there is a problem with overriding in Litestar and I find out, that object in Provide is cloned, because it's id is differ
#119

@vrslev
Copy link
Collaborator

vrslev commented Nov 18, 2024

Actually, I encountered this exact issue recently)

Is it safe to redefine deepcopy method to forbid providers cloning?

I guess so, I don’t see any potential issues with it in our context.

@lesnik512 lesnik512 marked this pull request as ready for review November 18, 2024 18:38
tests/integrations/test_litestar_di.py Outdated Show resolved Hide resolved
tests/integrations/test_litestar_di.py Outdated Show resolved Hide resolved
@lesnik512 lesnik512 merged commit d9dfff9 into main Nov 19, 2024
4 checks passed
@lesnik512 lesnik512 deleted the 119-bug-provider-overriding-does-not-works-with-litestar branch November 19, 2024 13:49
@@ -22,6 +24,9 @@ def __init__(self) -> None:
super().__init__()
self._override: typing.Any = None

def __deepcopy__(self, *_: object, **__: object) -> typing_extensions.Self:
Copy link

Choose a reason for hiding this comment

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

I think very good decision will be to left here comment with reference to issue and reference to litestar behaviour. Because at some point it may be not so easy to understand why this decision is been made

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, thank you

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.

Bug: provider overriding does not works with Litestar
4 participants