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

Lazy connectivity for Temporal client and healthCheck #224

Closed
Tracked by #45
bergundy opened this issue Aug 3, 2022 · 8 comments · Fixed by #320 or #411
Closed
Tracked by #45

Lazy connectivity for Temporal client and healthCheck #224

bergundy opened this issue Aug 3, 2022 · 8 comments · Fixed by #320 or #411
Assignees
Milestone

Comments

@bergundy
Copy link
Member

bergundy commented Aug 3, 2022

SDK should have a lazy and eager way to create a client and expose a healthcheck method

ref: #223

@wolfy-j
Copy link
Collaborator

wolfy-j commented Oct 18, 2022

The SDK healthcheck goes though the already open client on RR side. A.k.a. it should aready work as expected and not create any excessive connections.

@bergundy
Copy link
Member Author

Does PHP client support lazy and eager creation?
Does it call getSystemInfo on connection?
Is the gRPC health service exposed via the client?

We have all of that in the other SDKs.

@wolfy-j
Copy link
Collaborator

wolfy-j commented Nov 22, 2022

Not at the moment. We also don't have dial() routine in PHP, it happens inside underlyling C library.

@roxblnfk We will work on RFC based on temporalio/features#45

@roxblnfk roxblnfk added this to the 2.7.0 milestone Dec 12, 2023
@roxblnfk
Copy link
Collaborator

roxblnfk commented Dec 13, 2023

With the release of 2.7.0, interceptor functionality will be introduced. Along with this, the SystemInfoInterceptor is added, which calls getSystemInfo() and getCapabilities() on the first client request.

This interceptor is not added by default, but we can do this - add SystemInfoInterceptor if the user has not specified their list of interceptors for client calls. If the user wants to disable the interceptor, they can simply pass an empty list.
@wolfy-j @Sushisource what do you think about this?

@roxblnfk roxblnfk linked a pull request Dec 13, 2023 that will close this issue
@roxblnfk
Copy link
Collaborator

Regarding gRPC HealthCheck - it's not supported by the PHP gRPC extension.
https://grpc.io/docs/guides/health-checking/#language-support

@wolfy-j
Copy link
Collaborator

wolfy-j commented Dec 13, 2023

@bergundy do we expect HealthCheck mostly in Activity and WF pollers or client code as well? We do have health checks for all the pollers, but PHP does not have a BG check mechanism for clients.

@bergundy
Copy link
Member Author

In other languages we have a lazy method to create a connection (client) that doesn't make any calls to the server, and an eager connection method that fails if getSystemInfo fails.
We also expose the gRPC heath service in the client for users that want to call that.

That's all I requested in this issue.

@wolfy-j
Copy link
Collaborator

wolfy-j commented Dec 13, 2023

Eager connect on demand (lazy) - done (in fact in PHP this is the default way how GRPC works)
getSystemInfo - done with 2.7

I think we are good there.

@roxblnfk roxblnfk modified the milestones: 2.7.0, 2.8.0 Dec 14, 2023
@roxblnfk roxblnfk modified the milestones: 2.8.0, 2.9.0 Mar 7, 2024
@roxblnfk roxblnfk linked a pull request Apr 1, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants