-
Notifications
You must be signed in to change notification settings - Fork 808
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 support for MongoDB.Driver version 3 #2324
Conversation
@@ -8,7 +8,7 @@ | |||
</PropertyGroup> | |||
|
|||
<ItemGroup> | |||
<PackageReference Include="MongoDB.Driver" Version="2.28.0" /> | |||
<PackageReference Include="MongoDB.Driver" Version="[2.28.0,3.0.0)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 2.30.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the top comment. This is outdated now. We will only support the 3+ version going forward.
… MongoDB.Driver.Core assembly. Handle this breaking change by splitting our package into 2, one for each major version. 1. For the current HealthChecks.MongoDb package, we put a NuGet version limit on our dependency: [2.28.0,3.0.0). This way people won't be able to update to the 3.0.0 version, which will break their app. 2. We add a new, forked component named HealthChecks.MongoDb.v3 which will have a dependency on 3.0.0 and contains updates so the health checks will work with v3. People who explicitly want to use version 3 can opt into using this package. 3. When the next major version of HealthChecks ships, we can "swap" the dependencies around. The HealthChecks.MongoDb package will be updated to depend on version 3 of MongoDB.Driver. If MongoDB.Driver v2.x is still in support, we can create HeatlhChecks.MongoDb.v2 which has the dependency limit [2.28.0,3.0.0) and works with the version 2 of MongoDB.Driver. HealthChecks.MongoDb.v3 will be dead-ended. Fix Xabaril#2322
…ongoDB.Driver v3.0.0.
d23edfd
to
14e7d9d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2324 +/- ##
==========================================
- Coverage 66.88% 66.32% -0.56%
==========================================
Files 268 254 -14
Lines 8730 8571 -159
Branches 631 613 -18
==========================================
- Hits 5839 5685 -154
+ Misses 2723 2722 -1
+ Partials 168 164 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -1,7 +1,8 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
|
|||
<PropertyGroup> | |||
<TargetFrameworks>$(DefaultLibraryTargetFrameworks)</TargetFrameworks> | |||
<!-- Match the TFMs of MongoDB.Driver --> | |||
<TargetFrameworks>$(DefaultNetCoreApp);netstandard2.1;net472</TargetFrameworks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should simply drop the NS support?
<TargetFrameworks>$(DefaultNetCoreApp);netstandard2.1;net472</TargetFrameworks> | |
<TargetFrameworks>$(DefaultNetCoreApp);net472</TargetFrameworks> | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the underlying MongoDB.Driver
library supports netstandard2.1
, I feel like the health check library should as well. It would be a pity to block someone for no reason.
For example, I think Unity supports netstandard2.1 - https://docs.unity3d.com/6000.0/Documentation/Manual/dotnet-profile-support.html
@adamsitnik - I believe this is ready for re-review. I have addressed all the feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @eerhardt !
Thanks a lot everyone! @adamsitnik, is it possible to estimate when this will be available as nuget package. Or can we somehow get the preview package? Background, we have a release coming and need to get our MongoDb health checks green, the one way or the other. |
I am aiming for next week, but it depends on the availability of @unaizorrilla |
MongoDB.Driver version 3 made a major breaking change by removing the MongoDB.Driver.Core assembly.
Handle this breaking change by splitting our package into 2, one for each major version.1. For the current HealthChecks.MongoDb package, we put a NuGet version limit on our dependency: [2.28.0,3.0.0). This way people won't be able to update to the 3.0.0 version, which will break their app.2. We add a new, forked component named HealthChecks.MongoDb.v3 which will have a dependency on 3.0.0 and contains updates so the health checks will work with v3. People who explicitly want to use version 3 can opt into using this package.3. When the next major version of HealthChecks ships, we can "swap" the dependencies around. The HealthChecks.MongoDb package will be updated to depend on version 3 of MongoDB.Driver. If MongoDB.Driver v2.x is still in support, we can create HeatlhChecks.MongoDb.v2 which has the dependency limit [2.28.0,3.0.0) and works with the version 2 of MongoDB.Driver. HealthChecks.MongoDb.v3 will be dead-ended.Similar changes as Add support for RabbitMQ.Client version 7. (Xabaril/AspNetCore.Diagnostics.HealthChecks#2323).Handle this breaking change by updating to the new v3 version in HealthChecks
v9.x
release. According to https://www.mongodb.com/community/forums/t/net-driver-2-30-0-released/301201, MongoDB.Driverv2.x
will no longer be supported:Because of this, the HealthChecks should only need to support v3+ going forward. If someone needs to use the
v2.x
version, they can continue to use AspNetCore.HealthChecks.MongoDbv8.x
.Fix #2322
Please make sure you've completed the relevant tasks for this PR, out of the following list: