search

dapr / python-sdk

Dapr SDK for Python
Apache License 2.0
222 stars 125 forks source link

Check outbound healthz endpoint on sidecar wait #611

Closed mukundansundar closed 7 months ago

mukundansundar commented 11 months ago

Feature request

Similar to dotnet sdk check the outbound healthz endpoint for checking if sidecar is ready. https://github.com/dapr/dotnet-sdk/blob/master/src/Dapr.Client/DaprClientGrpc.cs#L1790

Even if the port is open, Dapr might not be ready yet.

Current behavior

Python only waits to check if the port is available and socket connection can be created. https://github.com/dapr/python-sdk/blob/master/dapr/clients/grpc/client.py#L1431

RELEASE NOTE:

mukundansundar commented 11 months ago

@dapr/maintainers-python-sdk For this one, can I modify the existing wait method in the gRPC clients (sync and async), to directly check against the HTTP endpoint "/v1.0/healthz/outbound" instead of the the socket check that we do right now. This will change the behavior of wait method, but will not be a breaking change since this was supposed to be the actual health check and current implementation is considered a bug.

berndverst commented 11 months ago

Making HTTP calls within the gRPC client might be unexpected behavior for some users:

The good news is that there is already precedent for using both protocols at the same time: The service invocation client in the Dapr Python SDK defaults to HTTP unless otherwise configured. And Actors are HTTP only right now! Officially the recommended way is to perform service invocation with native libraries however, rather than using the Dapr SDK. For folks that truly only use gRPC, or do not use service invocation however this could theoretically be a breaking change if they have misconfigured their (unused) Dapr HTTP Port or the Python SDK.

I would prefer if the runtime were to implement a native gRPC health method. But short of that, I think we can create a blocking utility method that calls the HTTP outbound health endpoint and retries until that passes. I do not want this HTTP code to live directly inline within the gRPC code, hence the utility method.

I suggest we add a new helpers.py under dapr/clients/http which performs this healthz check. And then you import that function from the wait method in the grpc client as you mentioned. Additionally, you also need to use this in the Dapr Actor Client and Dapr Service Invocation Client.

mukundansundar commented 11 months ago

Additionally, you also need to use this in the Dapr Actor Client and Dapr Service Invocation Client.

Based on the current usages, I only see the wait method being called in examples. Can you tell me why we need to use this in Actor Client and Service Invocation Client?

berndverst commented 10 months ago

Additionally, you also need to use this in the Dapr Actor Client and Dapr Service Invocation Client.

Based on the current usages, I only see the wait method being called in examples. Can you tell me why we need to use this in Actor Client and Service Invocation Client?

It might be nice for anything which talks to the sidecar to first wait to make sure Dapr is ready. So if it's missing it should be added I think.

elena-kolevska commented 7 months ago

Done in https://github.com/dapr/python-sdk/pull/670