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

feat(pubsub): support payload wrapping for push subs #8292

Merged
merged 6 commits into from
Jul 24, 2023

Conversation

hongalex
Copy link
Member

@hongalex hongalex commented Jul 19, 2023

This PR introduces the new payload unwrapping feature to the handwritten admin client.

Docs here: https://cloud.google.com/pubsub/docs/payload-unwrapping

Changes to vet.sh are because including pubsub.PubsubWrapper is intentional. PubsubWrapper is the name included in the proto. The alternative would be defining it as pubsub.PushConfigPubsubWrapper which seems perhaps more verbose and inconsistent with what we've already been doing (like with pubsub.OIDCToken)[https://pkg.go.dev/cloud.google.com/go/pubsub#OIDCToken]

@hongalex hongalex requested review from a team and shollyman as code owners July 19, 2023 23:40
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: pubsub Issues related to the Pub/Sub API. labels Jul 19, 2023
@@ -498,6 +498,11 @@ func (s *GServer) CreateSubscription(_ context.Context, ps *pb.Subscription) (*p
}
if ps.PushConfig == nil {
ps.PushConfig = &pb.PushConfig{}
} else if ps.PushConfig.Wrapper == nil {
// Wrapper should default to PubsubWrapper.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defaulting to nil is also acceptable and is treated as PubsubWrapper.

Copy link
Member Author

@hongalex hongalex Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just implementing that behavior in the fake server we provide. The client doesn't know about the default and should default to nil

Copy link
Contributor

@shollyman shollyman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's times like this I wish oneof yielded a proto type we could embed, but the isWrapper() interface seems sufficient here.

@hongalex hongalex enabled auto-merge (squash) July 24, 2023 17:56
Copy link
Contributor

@noahdietz noahdietz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stamp for vet.sh change.

@hongalex hongalex merged commit fd49db5 into googleapis:main Jul 24, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants