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: implement CreateOrUpdateStream function #1395

Merged
merged 11 commits into from
Oct 7, 2023
Merged

Conversation

samanebi
Copy link
Contributor

After using JetStream on a private project I felt the need for CreateOrUpdateStream like CreateOrUpdateConsumer. This function updates if stream exists, otherwise it creates the stream.

@piotrpio piotrpio self-requested a review September 12, 2023 14:18
@piotrpio
Copy link
Collaborator

Thanks you for creating the PR! The difference between creating streams and consumers is server side - for consumers, there used to be a single API for both creating and updating a consumer, thus it was a single operation on the client (that will change with the upcoming server release though, and we will be adding CreateConsumer and UpdateConsumer to the client as well). For streams, there are 2 separate APIs which means we can do safe creates and safe updates, which should usually be the preferred way.

That said, I am not strongly against adding CreateOrUpdateStream as convenience method, however that should probably be then reflected in other client libraries as well. Let me get back to you with this.

@samanebi
Copy link
Contributor Author

Thanks you for creating the PR! The difference between creating streams and consumers is server side - for consumers, there used to be a single API for both creating and updating a consumer, thus it was a single operation on the client (that will change with the upcoming server release though, and we will be adding CreateConsumer and UpdateConsumer to the client as well). For streams, there are 2 separate APIs which means we can do safe creates and safe updates, which should usually be the preferred way.

That said, I am not strongly against adding CreateOrUpdateStream as convenience method, however that should probably be then reflected in other client libraries as well. Let me get back to you with this.

Thanks :) I would be happy to help to implement this function in other clients

@MohsenKasraeifar
Copy link

this feature is going to be a very very useful feature and will use it definitely.

@HosseinEmadi
Copy link

Cool feature. Bravo @samanebi

Copy link
Collaborator

@piotrpio piotrpio left a comment

Choose a reason for hiding this comment

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

The feature looks good and we will add it! I only have a few suggestions.

jetstream/jetstream.go Outdated Show resolved Hide resolved
jetstream/test/jetstream_test.go Outdated Show resolved Hide resolved
jetstream/test/jetstream_test.go Outdated Show resolved Hide resolved
jetstream/jetstream.go Show resolved Hide resolved
@samanebi
Copy link
Contributor Author

All requested changes has been applied
@piotrpio

@samanebi
Copy link
Contributor Author

samanebi commented Oct 3, 2023

I am wondering when do you plan on merging this PR ?
@piotrpio

@samanebi
Copy link
Contributor Author

samanebi commented Oct 7, 2023

I am wondering when do you plan on merging this PR ? @piotrpio

Is there any update on this? I have done the suggested changes and also re requested review to @piotrpio
@piotrpio
@Jarema
@derekcollison

Copy link
Collaborator

@piotrpio piotrpio left a comment

Choose a reason for hiding this comment

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

LGTM!

@piotrpio
Copy link
Collaborator

piotrpio commented Oct 7, 2023

@samanebi Sorry for the delay, looks good! It'll be a part of the next release.

@piotrpio piotrpio merged commit e0f193c into nats-io:main Oct 7, 2023
1 check passed
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.

None yet

4 participants