Upgrade Google.Cloud.PubSub.V1 library

ReubenBond commented 2 years ago

We are currently referencing Google.Cloud.PubSub.V1 version 1.0.0-beta13. The latest version is 2.7.0 and we should update. The update is not straight-forward and may require some changes to configuration so that custom endpoints continue to be supported. Likely, the best route is to allow end-developers configure the PublisherClient/SubscriberClient themselves using a delegate on the relevant options type.

jeremylcarter commented 2 years ago

I've had a cursory glance at this and found the following:

Move from PublisherClient to PublisherServiceApiClient
Move from SubscriberClient to SubscriberServiceApiClient
Method PullAsync on SubscriberServiceApiClient has changed where the parameter boolean returnImmediately is now deprecated. See https://cloud.google.com/dotnet/docs/reference/Google.Cloud.PubSub.V1/latest/Google.Cloud.PubSub.V1.SubscriberServiceApiClient#Google_Cloud_PubSub_V1_SubscriberServiceApiClient_PullAsync_Google_Cloud_PubSub_V1_SubscriptionName_System_Boolean_System_Int32_Google_Api_Gax_Grpc_CallSettings_

Details about this boolean are:

Optional. If this field set to true, the system will respond immediately even if it there are no messages available to return in the Pull response. Otherwise, the system may wait (for a bounded amount of time) until at least one message is available, rather than returning no messages. Warning: setting this field to true is discouraged because it adversely impacts the performance of Pull operations. We recommend that users do not set this field.

ReturnImmediately is now obsolete (circa March 2020) - https://github.com/googleapis/google-cloud-dotnet/pull/4609

ServiceEndpoint has been removed see https://github.com/googleapis/google-cloud-dotnet/blob/e04d0561688dec66080c8b7a96bb3ba021a035cb/docs/devsite-help/breaking-gax2.md

Questions:

Why are we setting it to true in the first place? Is true the desired behaviour?
Is it ok if the GetMessages method of PubSubDataManager awaits a while?
If it is ok that the PullAsync no longer returns on no messages should we be concerned about timeouts?

ghost commented 1 year ago

We've moved this issue to the Backlog. This means that it is not going to be worked on for the coming release. We review items in the backlog at the end of each milestone/release and depending on the team's priority we may reconsider this issue for the following milestone.

jeremylcarter commented 11 months ago

Did you want me to look at this again @ReubenBond ?

ReubenBond commented 11 months ago

That would be appreciated! There is an existing PR here, but tests are failing in CI: https://github.com/dotnet/orleans/pull/8572

dotnet / orleans

Upgrade Google.Cloud.PubSub.V1 library #7509