Skip to content

use subjects for generating topic name instead of name from streamconfig#33

Open
grumblebrian wants to merge 1 commit into
ThreeDotsLabs:masterfrom
grumblebrian:master
Open

use subjects for generating topic name instead of name from streamconfig#33
grumblebrian wants to merge 1 commit into
ThreeDotsLabs:masterfrom
grumblebrian:master

Conversation

@grumblebrian

Copy link
Copy Markdown

Motivation / Background

The JetStream Publish function uses streamConfig.Name for the topic when marshaling the message, when it should be using streamConfig.Subjects. This is not a problem when using defaultStreamConfigurator since that sets the passed in topic as the name while also setting the first Subjects item to the topic. However, when using a custom (existing) stream config it is likely that the name will be different from the topic. This is because topics typically contain periods to reflect a topic hierarchy and periods cannot be used in stream names. Thus, a custom jetstream.StreamConfig likely appears similar to the following:

jetstream.StreamConfig{
    Name: "COMMAND_TEST",
    Subjects: []string{"command.test"},
    Storage: jetstream.FileStorage,
    Retention: 72 * time.Hour,
}

Typically, stream names are capitalized but that is not required.

Details

This was a simple change to use streamConfig.Subjects[0] instead of streamConfig.Name for the topic when calling the Marshal function. This does not change the default behavior when the ConfigureStream PublisherConfig option is not used when creating a JetStream publisher since defaultStreamConfigurator sets both Name and Subjects with the passed-in topic.

Checklist

The resources of our team are limited. There are a couple of things that you can do to help us merge your PR faster:

  • I wrote tests for the changes.
  • All tests are passing.
    • If you are testing a Pub/Sub, you can start Docker with make up.
    • You can start with make test_short for a quick check.
    • If you want to run all tests, use make test.
  • Code has no breaking changes.
  • (If applicable) documentation on watermill.io is updated.

@christian-gama

Copy link
Copy Markdown

This is indeed important. I often have to manually setup the streams due to this watermill behavior.

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.

2 participants