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

Added 3 overloads of AddEventStoreClient #238

Merged

Conversation

NielsPilgaard
Copy link
Contributor

@NielsPilgaard NielsPilgaard commented Jan 20, 2023

Hi there 👋

This PR adds 3 new overloads of AddEventStoreClient that provide access to the IServiceProvider, allowing easy configuration across environments. This is very useful when using EventStore on different environments, that use different EventStores.

They allow constructing the following using the ServiceProvider:

  • Uri address
  • string connectionString
  • Action<EventStoreClientSettings configureSettings

I've also added:

  • 2 new private methods that do the actual registration
  • tests for all 3 public methods

I currently have one of these extension methods running in production, and it works very well in combination with a configuration manager such as Azure App Configuration, or Azure Key Vault.

These new methods provide access to the `IServiceProvider`, allowing easy configuration across environments.
This is very useful when using EventStore on different environments, that use different EventStores.

I currently have this code running in production, and it works very well in combination with a configuration manager such as Azure App Configuration, or Azure Key Vault.

They allow constructing the following using the ServiceProvider:
- Uri address
- string connectionString
- Action<EventStoreClientSettings` configureSettings

I've also added:
- 2 new private methods that do the actual registration
- tests for all 3 public methods
@NielsPilgaard
Copy link
Contributor Author

@tambeau tambeau requested a review from ylorph January 24, 2023 13:20
@NielsPilgaard
Copy link
Contributor Author

The PR linked above has been merged, so Client development project automations for pull requests ought to work now :)

Also renamed tests to be more human-readable
timothycoleman
timothycoleman previously approved these changes Feb 16, 2023
Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

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

That's great, thanks very much!

This fixes publish failing with the error `error NETSDK1194: The "--output" option isn't supported when building a solution.`

The `--output` flag has been deprecated, the recommended approach is to use `/p:PublishDir=..` instead.
Reference: dotnet/performance#2774
@NielsPilgaard
Copy link
Contributor Author

NielsPilgaard commented Feb 16, 2023

The publish workflow failed, commit 5f606de in this PR ought to fix it though :)

@NielsPilgaard
Copy link
Contributor Author

@alexeyzimarev
Copy link
Member

If you'd like I could remove net5.0 from the various actions.

That would be great!

@thefringeninja
Copy link
Contributor

@NielsPilgaard we'll remove net5 in a separate PR

@NielsPilgaard
Copy link
Contributor Author

Alrighty 👍 I wouldn't mind making that PR either, but I totally understand if you'd prefer a change like that to be made in-house.

.github/workflows/publish.yml Outdated Show resolved Hide resolved
@NielsPilgaard
Copy link
Contributor Author

I've committed a fix for my newest pipeline bug 😅 the packages folder is now created if it doesn't exist, just before running dotnet pack.

Copy link
Contributor

@thefringeninja thefringeninja left a comment

Choose a reason for hiding this comment

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

publishing artifacts will probably fail but we can address that in a future PR

Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

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

👍

@timothycoleman timothycoleman merged commit 4384549 into EventStore:master Feb 27, 2023
@NielsPilgaard NielsPilgaard deleted the provider-in-registration branch February 27, 2023 16:56
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.

4 participants