Skip to content

Comments

Di pattern change#2

Draft
panykd wants to merge 5 commits intomasterfrom
di-pattern-change
Draft

Di pattern change#2
panykd wants to merge 5 commits intomasterfrom
di-pattern-change

Conversation

@panykd
Copy link
Owner

@panykd panykd commented Nov 9, 2019

No description provided.

Copy link
Collaborator

@Kermittt Kermittt left a comment

Choose a reason for hiding this comment

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

        public void ConfigureServices(IServiceCollection services)
        {
            services.AddOptions<RabbitMqOptions>().Bind(Configuration.GetSection("Orleans:RabbitMq"));
        }

        public void Configure(ISiloBuilder silo)
        {
            silo..UseRabbitMqStreams(StreamProviders.RabbitMq);
        }

Exception, line 45, RabbitMqOptions
Orleans.Runtime.OrleansConfigurationException: 'Missing required parameter 'HostName' on stream provider RabbitMq!'

https://github.com/Kermittt/squeaker-sam/tree/rabbit-di-pattern-change

/// </summary>
public static IClientBuilder UseRabbitMqStreams(this IClientBuilder builder, string name, Action<RabbitMqOptions> options)
{
return UseRabbitMqStream<DefaultBatchContainerSerializer>(builder, name, options);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a typo? Sometimes it seems to be plural and sometimes singular.
The name 'UseRabbitMqStream' does not exist in the current context

Copy link
Owner Author

Choose a reason for hiding this comment

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

The original methods (marked [Obsolete]) are singular. The new ones are plural

/// <summary>
/// Configure client to use RMQ persistent streams, using the <see cref="DefaultBatchContainerSerializer"/>.
/// </summary>
[Obsolete("Use 'UseRabbitMqSteams'")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another type here. Missing an r.
UseRabbitMqSteams

Copy link
Owner Author

Choose a reason for hiding this comment

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

fixing

@Kermittt
Copy link
Collaborator

I think I know why this couldn't find the HostName. I call ConfigureServices after Configure so that the EndpointOptions overwrite the settings from ConfigureEndpoints. Might have to rethink how I'm dealing with that one.
(Some annoying thing about having to call ConfigureEndpoints because it sets an ip address internally, even though I don't actually want to set the ports there.)

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