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

Add an overload to UseCosmos that takes a connection string #21088

Merged
merged 1 commit into from
Jun 14, 2020

Conversation

Marusyk
Copy link
Member

@Marusyk Marusyk commented May 30, 2020

Add an overload to UseCosmos that takes a connection string.

Fixes #20712

For now, I have some problems with testing it. Could you please suggest if I'm on the right way? I'll continue to work on tests.

@Marusyk Marusyk marked this pull request as ready for review May 30, 2020 17:58
@AndriySvyryd
Copy link
Member

General approach looks good. You can put the tests in a new class similar to https://github.com/dotnet/efcore/blob/bde2b140d6f4cf94d6d1285d402941e20193ec60/test/EFCore.SqlServer.FunctionalTests/ConnectionSpecificationTest.cs

@AndriySvyryd
Copy link
Member

@Marusyk Are you still planning to work on this during June?

@Marusyk
Copy link
Member Author

Marusyk commented Jun 10, 2020

Yes, but I have a blocker. I cann't run the CosmosDB emulator on my PC.. So, I cann't add tests

@AndriySvyryd
Copy link
Member

@Marusyk Uninstall and reinstall the emulator, then right-click on the task bar icon and select "Reset Data...". If it still doesn't work then add the tests blindly and I'll fix them if necessary.

@Marusyk
Copy link
Member Author

Marusyk commented Jun 13, 2020

Add an overload that takes a single string rather than the URI and the key
It seems we cannot use a single string parameter because we also need to specify the database name.

The connection string to CosmosDB doesn't contains Database name

AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==

@AndriySvyryd

should I add something like:
UseCosmos(string connectionString, string database)

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Jun 13, 2020

should I add something like:
UseCosmos(string connectionString, string database)

Yes, all overloads should have the Database name parameter

@Marusyk
Copy link
Member Author

Marusyk commented Jun 13, 2020

Added only one test for now.
Not sure about changes in GetServiceProviderHashCode and PopulateDebugInfo methods from CosmosOptionsExtension file.

@Marusyk
Copy link
Member Author

Marusyk commented Jun 13, 2020

Could you please help with the build? Don't understand what's wrong, I got "Build succeeded." on the local machine.

@AndriySvyryd
Copy link
Member

@Marusyk Rebase your changes on latest master

Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

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

Squash the commits

@AndriySvyryd AndriySvyryd merged commit d724d08 into dotnet:master Jun 14, 2020
@AndriySvyryd
Copy link
Member

@Marusyk Thanks for your contribution!

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.

Add support for Cosmos DB Connection strings
2 participants