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

fix(pubsub): add deadletter and retries handling in the fake pubsub #5320

Merged
merged 6 commits into from Jan 18, 2022

Conversation

yottta
Copy link
Contributor

@yottta yottta commented Jan 8, 2022

Client

PubSub

Go Environment

go version go1.16.6 darwin/amd64

Description

The PubSub library offers a testing server that is still in beta and therefore does not have all the functionalities.
This ticket is about improving the PubSub testing server to handle correctly Deadletters and retries.

Expected behavior

When a Subscription is created with a DeadLetter and retries configuration, an unacked message is received from the subscription only as many times as specified in the retries configuration.

Actual behavior

When a Subscription is created with a DeadLetter configuration, an unacked message is never sent to deadletter and is received from the subscription indefinitely.

@yottta yottta requested review from a team as code owners January 8, 2022 13:07
@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 Jan 8, 2022
@yottta
Copy link
Contributor Author

yottta commented Jan 8, 2022

Solves #5319

@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 14, 2022
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 14, 2022
Copy link
Member

@hongalex hongalex left a comment

Choose a reason for hiding this comment

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

Thanks for drafting this PR. I had a few minor suggestions but otherwise looks good!

pubsub/pstest/fake.go Show resolved Hide resolved
pubsub/pstest/fake.go Outdated Show resolved Hide resolved
pubsub/pstest/fake_test.go Show resolved Hide resolved
yottta and others added 2 commits January 15, 2022 09:42
…dead letter policy is set

Co-authored-by: Alex Hong <9397363+hongalex@users.noreply.github.com>
if m.deliveries != nil {
deliveries = *m.deliveries
}
s.deadLetterTopic.publish(m.proto.Message, &Message{
Copy link
Contributor Author

@yottta yottta Jan 15, 2022

Choose a reason for hiding this comment

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

Here I doubt that I need to use the original message properties (deliveries and acks) since the publish method is not allowing the proto values as well for these properties.
This is about the changes that you suggested around m.proto.DeliveryAttempt.
Also you can check fake_test.go#382

@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 18, 2022
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jan 18, 2022
@hongalex hongalex merged commit 116a610 into googleapis:main Jan 18, 2022
BrennaEpp pushed a commit to BrennaEpp/google-cloud-go that referenced this pull request Jan 19, 2022
…oogleapis#5320)

* fix(pubsub): add deadletter and retries handling in the fake pubsub server

* fix(pubsub): Sync delivery attempt field here before incrementing if dead letter policy is set

Co-authored-by: Alex Hong <9397363+hongalex@users.noreply.github.com>

* fix(pubsub): delivery attempts to proto as well; typos

Co-authored-by: Alex Hong <9397363+hongalex@users.noreply.github.com>
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.

None yet

3 participants