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

Pass to annotation Provider instead of FromComponent("component name") #168

Closed
Olegt0rr opened this issue Jun 8, 2024 · 2 comments
Closed
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@Olegt0rr
Copy link
Contributor

Olegt0rr commented Jun 8, 2024

Motivation

It is inconvenient to use additional strings/constants for the sake of components.

class TelegramProvider(Provider):
    component = "Telegram"
    ...

    @provide(scope=Scope.REQUEST)
    async def get_user(self, telegram_object: TelegramObject) -> User:
        return telegram_object.from_user

@router.message()
async def handler(
    message: Message,
    user: Annotated[User, FromComponent("Telegram")],
) -> None: ...

Suggestion

It would be convenient to add MyProvider to the annotation, and then use its component field under the hood of the library.

@router.message()
async def handler(
    message: Message,
    user: Annotated[User, TelegramProvider],
) -> None: ...

Additional

This solution has one more advantage: we get the opportunity to move to the provider in 1 click (e.g. to edit it).
This is not possible in the current implementation (but it's possible in fastapi.Depends)

This solution also helps to avoid pyCharm bug with brackets highlighting: https://youtrack.jetbrains.com/issue/PY-57155/

@Olegt0rr Olegt0rr changed the title Pass to annotation Provider instead of `FromComponent("component name") Pass to annotation Provider instead of FromComponent("component name") Jun 8, 2024
@Tishka17
Copy link
Member

Tishka17 commented Jun 8, 2024

Implementing this will break the ability to multiple dependency graphs for the same target dependencies. In short: there will be no DI(!)

Also, it is not possible to implement, because there can be multiple providers at a same time for single component.

I do not think, components is for every day case, it is more a for specific case when we need to isolate subgraphs. Event in those cases, I expect to use them for internal parts of graph and use default component in view functions

@Tishka17 Tishka17 closed this as completed Jun 8, 2024
@Tishka17 Tishka17 added enhancement New feature or request invalid This doesn't seem right wontfix This will not be worked on and removed invalid This doesn't seem right labels Jun 8, 2024
@Olegt0rr
Copy link
Contributor Author

Olegt0rr commented Jun 9, 2024

because there can be multiple providers at a same time for single component.

Of course it can be!
But because visibility is open within the component, nothing prevents any provider associated with the component from passing

This still works as expected, anyway

@router.message()
async def handler(
    message: Message,
    user: Annotated[User, FromComponent(TelegramProvider.component)],
) -> None: ...

So I just ask for shortcutting it

Just for fun accepted to use Provider into hint_to_dependency_key function.
All existing tests passed well and proposed syntax become available

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants