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

Remove autoAck from Persistent Subscriptions #175

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

hayley-jean
Copy link
Member

Removes autoAck option when connecting to Persistent Subscriptions. Going forward, subscribers will need to explicitly ack/nack messages using subscription.Ack(evnt);.

See the docs here for instructions on how to ack/nack manually.

Fixes EventStore/EventStore#3113

@hayley-jean hayley-jean self-assigned this Jan 3, 2022
@hayley-jean hayley-jean force-pushed the hayley-jean/remove-auto-ack branch 3 times, most recently from a261b3f to e9ea2b1 Compare January 4, 2022 13:41
@hayley-jean hayley-jean marked this pull request as ready for review January 4, 2022 13:41
oskardudycz
oskardudycz previously approved these changes Jan 6, 2022
Copy link
Contributor

@oskardudycz oskardudycz left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@hayley-jean Regression / regression-ubuntu-18.04/EventStore.Client.Streams-20.6.0 tests are failing, I'm not sure it that's expected.

- Obsolete the SubscribeAsync method, use SubscribeToStreamAsync instead
- Throw an error if using SubscribeAsync with autoAck=true
- Add SubscribeToStreamAsync without autoAck, messages must be manually
  acked/nacked
- Remove autoAck from SubscribeToAllAsync
- Update samples
- Remove auto-ack tests
@hayley-jean hayley-jean force-pushed the hayley-jean/remove-auto-ack branch from ef7a88e to 3186014 Compare January 6, 2022 13:12
Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

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

looks good 👍

@hayley-jean
Copy link
Member Author

@hayley-jean Regression / regression-ubuntu-18.04/EventStore.Client.Streams-20.6.0 tests are failing, I'm not sure it that's expected.

The regression test is failing because of a previous regression to do with deleted streams, so it's expected

@hayley-jean hayley-jean merged commit b683c7d into master Jan 7, 2022
@hayley-jean hayley-jean deleted the hayley-jean/remove-auto-ack branch January 7, 2022 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SubscribeAsync to set autoAck to false as default
3 participants