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

Typing of HealthServicer.set #30

Open
jonyscathe opened this issue Apr 11, 2022 · 6 comments · May be fixed by #40
Open

Typing of HealthServicer.set #30

jonyscathe opened this issue Apr 11, 2022 · 6 comments · May be fixed by #40

Comments

@jonyscathe
Copy link

Hi,
Just curious about a change in the last release (1.24.9)

After this change, I'm finding it hard to set the status of services without a mypy error.

The code straight from the grpc examples https://github.com/grpc/grpc/blob/master/examples/python/xds/server.py:

    # Mark all services as healthy.
    health_pb2_grpc.add_HealthServicer_to_server(health_servicer, server)
    for service in services:
        health_servicer.set(service, health_pb2.HealthCheckResponse.SERVING)

Now causes a mypy error of:

Argument 2 to "set" of "HealthServicer" has incompatible type "ValueType"; expected "ServingStatus"  [arg-type]

I think argument 2 should be a HealthCheckResponse.ServingStatus.ValueType ?
Defined in grpc_health-stubs/v1/health.pyi

This never used to cause a problem with health_pb2 stubs missing, but now they are there I am getting this error.
Or if I am wrong about that arguments type, how do I now set a servicer status without a mypy error?

@gitpushdashf
Copy link
Contributor

I'm running into this as well. Thanks for opening the issue!

@shabbyrobe
Copy link
Owner

Best I can do here at the moment is to revert the offending commit. I've pushed 1.24.10 to PyPi for this now. Please let me know if this "fixes" your issue, I'll leave the issue open for a bit.

@shabbyrobe shabbyrobe reopened this Apr 16, 2022
@gitpushdashf
Copy link
Contributor

This isn't working for me, even though in Github it looks correct.

root@30514da15ae0:/# ls /usr/local/lib/python3.10/site-packages/grpc_health-stubs/v1
__init__.pyi  health.pyi  health_pb2.pyi  health_pb2_grpc.pyi

If I install grpc-stubs 1.24.8:

root@0526e361c909:/# ls /usr/local/lib/python3.10/site-packages/grpc_health-stubs/v1
__init__.pyi  health.pyi

@jonyscathe
Copy link
Author

Yep - still not working for me either.

Despite it looking like health_pb2.pyi and health_pb2_grpc.pyi were deleted from 1.24.9 to 1.24.10 they are still there when I run `pip install grpc-stubs==1.24.10' and only disappear when I run 'pip install grpc-stubs==1.24.8'

Running a pip uninstall and pip install still doesn't get rid of those files - really not sure what has gone wrong here.

Those two files are still in the grpc_stubs-1.24.10.dist-info/RECORD as well:
grpc_health-stubs/v1/health_pb2.pyi,sha256=yxZ9PQu3oKjCmaCNFAFz4vkHRN014EYxBnrcPz1zPbc,2101
grpc_health-stubs/v1/health_pb2_grpc.pyi,sha256=_-N-BDkiOXBEwoE6sSQGDPyL_ECniOuvPJn9INHsPKY,1090

On the other hand - if I take either 1.24.9 or 1.24.10 and modify grpc_health-stubs/v1/health.pyi's line 39 from:
'''
def set(self, service: str, status: _health_pb2.HealthCheckResponse.ServingStatus) -> None:
'''
to
'''
def set(self, service: str, status: _health_pb2.HealthCheckResponse.ServingStatus.ValueType) -> None:
'''

Then I get no mypy errors and everything is fine - and I believe that that is the correct type for the set() function.

If you don't see any issues with this change I am happy to make a PR adding health_pb2.pyi and health_pb2_grpc.pyi back in and making that small change to health.pyi.
And I believe that will resolve this issue.

@shabbyrobe
Copy link
Owner

Please note that there are now fairly substantial new requirements for PRs to be accepted to this repo (see the README for details). If you're happy to meet those requirements when re-adding health_pb2.pyi and health_pb2_grpc.pyi, then by all means re-add them, but I think the minimum viable change is probably just to properly restore to 1.24.8, which it seems like I haven't quite done right by the sounds of it?

@jonyscathe
Copy link
Author

Fair enough - there could potentially be further issues with health_pb2.pyi or health_pb2_grpc.pyi and at this point in time I don't have sufficient time to go through them, adding test cases, etc to cover everything within those files.

And yes, it looks as though the 1.24.10 tag doesn't match what is on pypi for 1.24.10

@shabbyrobe shabbyrobe linked a pull request Apr 30, 2023 that will close this issue
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 a pull request may close this issue.

3 participants