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 object store watch to use NEW rather than ALL #659

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

caspervonb
Copy link
Collaborator

@caspervonb caspervonb commented Feb 3, 2025

This aligns with our behavior with nats.js, nats.go has some conflicting options regarding this behavior (https://github.com/nats-io/nats.go/blob/main/object.go#L1120-L1125)

Fixes #638

@caspervonb caspervonb requested a review from aricart February 3, 2025 10:16
@caspervonb caspervonb force-pushed the fix-object-store-include-history branch from 57e8b06 to 0f4c4a9 Compare February 3, 2025 10:42
@caspervonb caspervonb force-pushed the fix-object-store-include-history branch from 0f4c4a9 to 1ee1883 Compare February 12, 2025 12:05
@wallyqs
Copy link
Member

wallyqs commented Feb 13, 2025

Can you point to the similar code in nats.js?

@Jarema
Copy link
Member

Jarema commented Feb 13, 2025

It's a breaking change, as it changes the behaviour. We might be better off with a new method with good name, and good docs?

@caspervonb caspervonb force-pushed the fix-object-store-include-history branch from 1ee1883 to 3edc9b8 Compare February 13, 2025 12:14
@caspervonb
Copy link
Collaborator Author

caspervonb commented Feb 13, 2025

Can you point to the similar code in nats.js?

@wallyqs see https://github.com/nats-io/nats.js/blob/747c3e58e65369bb7ad7dfd58d263f8335e465f8/obj/src/objectstore.ts#L838-L844

It's a breaking change, as it changes the behaviour. We might be better off with a new method with good name, and good docs?

Possibly, or align with Go's other option. e.g include a kwarg for updates_only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

include_history for object store watch have no effect
3 participants