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

Cosmos: Resolve HttpClientFactory from DI if available #21274

Closed
AndriySvyryd opened this issue Jun 15, 2020 · 11 comments · Fixed by #25399
Closed

Cosmos: Resolve HttpClientFactory from DI if available #21274

AndriySvyryd opened this issue Jun 15, 2020 · 11 comments · Fixed by #25399
Labels
area-cosmos closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution type-enhancement
Milestone

Comments

@AndriySvyryd
Copy link
Member

https://docs.microsoft.com/en-us/dotnet/api/microsoft.azure.cosmos.cosmosclientoptions.httpclientfactory

@DaleyKD
Copy link

DaleyKD commented Aug 18, 2020

Yes, please. I was coming here to say how much I needed this.

Our use case is simple: when testing locally, we're in Docker. We fight all these blasted SSL cert issues with the Azure Cosmos DB Emulator that's hosted in Windows (the host). Version 3.12.0 of the SDK finally gave us the ability to circumvent that. However, EF Core is blocking us from going in that direction.

However, we won't be necessarily registering the HttpClientFactory in the DI for this; we're registering a CosmosClientOptions singleton to be used when we instantiate a new CosmosClient(). I know I saw a commit where you opened up a lot more options for the CosmosClientOptions, except not the HttpClientFactory. So this doesn't satisfy our needs quiet yet.

@dczychon
Copy link

I´m facing the same issue as @DaleyKD.
Can´t connect to local Comsos DB Emulator from EF Core running in Linux Docker container.
Always fails with "The SSL connection could not be established, see inner exception.".

@DaleyKD do you have managed to get a working workaround for this?

@DaleyKD
Copy link

DaleyKD commented Dec 9, 2020

@ajcvickers : I just got to look briefly at the CosmosDB Provider for EF Core 5 today and can already tell that it's so much better. Thank you.

Is there any way that in a near future release (pre 6.0), you can give us something in the CosmosDbContextOptionsBuilder to ignore the SSL cert errors? I realize it's a stop gap, but it's the ONE thing I'm waiting on that causes local development to be so darn slow.

@ajcvickers
Copy link
Member

@DaleyKD There won't be any GA/RTM release of EF Core before EF Core 6.0. You could consider using the daily builds or previews, which are generally very stable. We are considering fixing this for 6.0, or it may be fixed by a community PR, and the quickest ways to get that fix are going to be daily builds followed by previews.

@ajcvickers
Copy link
Member

@davidfowl @pranavkm @mkArtakMSFT From an ASP.NET perspective, should an application try to use the HttpClientFactory that is configured by ASP.NET to also access Cosmos? Or should a separate factory be used? Or does it not matter?

@davidfowl
Copy link
Member

Keep it separate, you don't know how the HttpClient is configured and its possible you don't want a poorly configured HttpClient to mess with cosmos? Do users ever need to set client settings for cosmos?

@dnperfors
Copy link
Contributor

The main reason for me to be able to configure the HttpClient, is to disable SSL verification for tests I run in the CI pipeline.

Since this is not (visible) on the roadmap for EFCore 6, would it help if I create a PR for it where I add a WithHttpClientFactory method on the CosmosDbContextOptionsBuilder?

@AndriySvyryd
Copy link
Member Author

AndriySvyryd commented Jul 29, 2021

@dnperfors You can try, the deadline for merging 6.0 features is August 15.

Since this is a CosmosClient option it also needs to be included in ICosmosSingletonOptions and used in CosmosDbOptionExtension.ExtensionInfo.GetServiceProviderHashCode and CosmosDbOptionExtension.ExtensionInfo.ShouldUseSameServiceProvider.

Because of this it's important to never instantiate Func<HttpClient> inside the AddDbContext lambda or OnConfiguring method as this will result in a different object each time a new context instance is created and this would cause a new internal service provider to be created. This needs to be documented clearly in WithHttpClientFactory

@DaleyKD
Copy link

DaleyKD commented Jul 30, 2021

@dnperfors : If you could get this to happen ASAP, that'd be glorious. I loathe using the Windows Cosmos DB Emulator solely because of the hassle I go thru to create SSL certs to then load into my Linux Docker container and hope it works. If I could just bypass the SSL validation for local dev, I'd be decently more efficient.

@dnperfors
Copy link
Contributor

@DaleyKD I am working on it, just need to test it some more and tidy up enforce creating the PR. I hope to get the change to finish it tonight or tomorrow.

dnperfors added a commit to dnperfors/efcore that referenced this issue Aug 3, 2021
@AndriySvyryd AndriySvyryd modified the milestones: Backlog, 6.0.0 Aug 7, 2021
@AndriySvyryd AndriySvyryd removed their assignment Aug 7, 2021
@AndriySvyryd AndriySvyryd added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed consider-for-current-release labels Aug 7, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0, 6.0.0-rc1 Aug 12, 2021
@ajcvickers ajcvickers modified the milestones: 6.0.0-rc1, 6.0.0 Nov 8, 2021
@markjbrown
Copy link

Hey @dnperfors thanks for your contributing to the Cosmos DB provider. I'd like to send you a Cosmos DB drink coaster and a sticker. If you could DM me on Twitter @markjbrown and send me your address, I'll get those shipped out to you.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-cosmos closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. community-contribution type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants