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

v3: Remove one of DefaultPath / DefaultTopic #289

Open
W1zzardTPU opened this issue Aug 3, 2024 · 3 comments
Open

v3: Remove one of DefaultPath / DefaultTopic #289

W1zzardTPU opened this issue Aug 3, 2024 · 3 comments
Milestone

Comments

@W1zzardTPU
Copy link

It's not worth it dragging both along in the code base forever

@zarusz
Copy link
Owner

zarusz commented Aug 3, 2024

So you suggest in v3 we leave the one DefaultPath() ?

The Path is more encompassing than the Topic or Queue name hence my tendency to leave Path terminology.

cc: @EtherZa

@W1zzardTPU
Copy link
Author

Yeah I would lean slightly towards "Path", too, as it's more neutral and less bus-specific jargon

@zarusz zarusz added this to the 3.0.0 milestone Aug 3, 2024
@EtherZa
Copy link
Contributor

EtherZa commented Aug 3, 2024

It may be worthwhile to have deeper look into the configuration as a whole in v3. I suspect a lot of it has come through organic growth (and perhaps operational bias) but there is generally a lot of repetition when configuring producers and consumers. This is exacerbated when a single queue is used for multiple message types resulting in a single consumer being created with multiple ConsumerSettings.

As an example, I use a local method similar to the one below to ease registration of messages/consumers being processed with shared subscriptions on Azure Service Bus.

void AddConsumer<TMessage, TConsumer>(string subscriptionName, int instances = 20, int prefetch = 100)
    where TConsumer : class, IConsumer<TMessage>
{
    bus.Produce<TMessage>(x => x.DefaultTopic(busOptions.Topic));
    bus.Consume<TMessage>(
        cfg =>
        {
            var type = typeof(TMessage);
            var typeName = resolver.ToName(type);
            cfg
                .Topic(busOptions.Topic)
                .SubscriptionName(subscriptionName)
                .Instances(instances)
                .PrefetchCount(prefetch)
                .WithConsumer<TConsumer>()
                .SubscriptionSqlFilter($"MessageType='{typeName}'", Hash(typeName));
        });
}

Each message that is registered in this way results in a new ConsumerBuilder being constructed even though the topic, subscription name, instance and prefetch count will be identical. In fact, if they were to differ, it would cause an issue with topology creation, etc. IMO it would be better to invert the logic here, such that the queue can be configured before configuring Produce and Consume. The same queue configuration should be used to configure all messages that it will be responsible for (or at least allow for a single instance to be referenced by each cfg => ...).

The current implementation is obviously succinct when each message is consumed by its own queue.

There are also other little niggles that make sense when you read through the code, but are not immediately obvious eg. mbb.AutoStartConsumersEnabled(false) being effective before mbb.AddChild(), but not afterwards.

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

No branches or pull requests

3 participants