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

Improve BaseApiClient example #86

Draft
wants to merge 2 commits into
base: v0.x.x
Choose a base branch
from

Conversation

llucax
Copy link
Contributor

@llucax llucax commented Sep 4, 2024

  • Improve the typing of the stub: the mypy-protobuf plugin generates a XxxAsyncStub class for each service, which have proper type information to be used with async. This still needs some hacks (type ignore) to make it work, but it's better than using the normal stubs and having to cast on each call.
  • Add channel_defaults.

The `mypy-protobuf` plugin generates a `XxxAsyncStub` class for each
service, which have proper type information to be used with async. This
still needs some hacks (type ignore) to make it work, but it's better
than using the normal stubs and having to cast on each call.

Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax requested a review from a team as a code owner September 4, 2024 16:30
@llucax llucax requested a review from Marenz September 4, 2024 16:30
@github-actions github-actions bot added the part:code Affects the code in general label Sep 4, 2024
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Sep 4, 2024
@llucax llucax self-assigned this Sep 4, 2024
@llucax llucax added this to the v0.7.0 milestone Sep 4, 2024
@llucax llucax disabled auto-merge September 4, 2024 16:34
@llucax llucax changed the title Improve the typing of the example in BaseApiClient Improve BaseApiClient example Sep 4, 2024
@llucax llucax added part:docs Affects the documentation and removed part:code Affects the code in general labels Sep 4, 2024
@llucax llucax marked this pull request as draft September 5, 2024 08:35
auto-merge was automatically disabled September 5, 2024 08:35

Pull request was converted to draft

@llucax
Copy link
Contributor Author

llucax commented Sep 5, 2024

Converting to draft because I'm finding issues at runtime with this approach, because the XxxAsyncStub doesn't really exist at runtime 😟

@llucax
Copy link
Contributor Author

llucax commented Sep 5, 2024

It looks like we need this accepted to make it work:

Otherwise we can't use the AsyncStub as a generic parameter when inheriting from BaseApiClient.

If this doesn't get updated and we want to still use the async stub, I guess we need to take out the stub from the BaseApiClient class and each sub-class will need to handle it manually on their own.

@llucax llucax added the status:blocked Other issues must be resolved before this can be worked on label Sep 5, 2024
@llucax
Copy link
Contributor Author

llucax commented Sep 5, 2024

A quick hack is:

    def __init__(...) -> None:
        super().__init__(
            server_url,
            microgrid_pb2_grpc.MicrogridStub,
            connect=connect,
            channel_defaults=channel_defaults,
        )
        self._async_stub: microgrid_pb2_grpc.MicrogridAsyncStub = self.stub  # type: ignore

And then just use self._async_stub internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd:skip-release-notes It is not necessary to update release notes for this PR part:docs Affects the documentation status:blocked Other issues must be resolved before this can be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant