digitalbazaar / bedrock-mongodb

Bedrock mongodb module
Apache License 2.0
2 stars 3 forks source link

Update required server version to >=4.2. #40

Closed davidlehn closed 4 years ago

davidlehn commented 4 years ago

Based on comment: https://github.com/digitalbazaar/bedrock-mongodb/pull/33#discussion_r434886994

codecov-commenter commented 4 years ago

Codecov Report

Merging #40 into driver3 will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           driver3      #40   +/-   ##
========================================
  Coverage    52.20%   52.20%           
========================================
  Files            4        4           
  Lines          385      385           
========================================
  Hits           201      201           
  Misses         184      184           
Impacted Files Coverage Δ
lib/config.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e1aec64...7eab010. Read the comment docs.

aljones15 commented 4 years ago

@davidlehn just so you know having serverVersion set to 4.2 caused the entire test project to crash on my computer with the error: could not initialize database. Lowering serverVersion to 4.0 solved the issue for me. So I have no problem with the tests being done on 4.2 on github actions, but it might be better to stick with >=4.0 as neither my mac or my linux box has mongo 4.2 yet. Oh yeah and good work! COOKIES!!!

davidlehn commented 4 years ago

@aljones15 Thank you for confirming the version check code works. The error output was poor. There is a error cause chain saying the server version check failed. I add a quick patch to help show that.

The purpose of this patch is to default to 4.2 since, as @mattcollier said, our other projects will start to use 4.2 features. Obviously installing 4.2 will be required if 4.2 features are used. This module itself doesn't seem to require 4,2 though. The version bump is just a default to avoid having to specify the required version in many other apps.

Others, please chime in. What minimum version should this module specify?

mattcollier commented 4 years ago

@aljones15 +1 to what @davidlehn said, we are going to start using 4.2 only features in our stuff. So that's going to be a requirement.

Here's some instructions on running mongodb in docker if it helps. I know you've also got datasets to be concerned about. So hopefully the upgrade is not too painful for you.