fix(NODE-3528): add support for snappy v7 #2947
Conversation
lib/core/connection/utils.js
Outdated
| if (bsonExtVersion.major >= 4) { | ||
| emitWarningOnce( | ||
| 'bson-ext version 4 and above does not work with the 3.x version of the mongodb driver' | ||
| ); |
There was a problem hiding this comment.
Since I added the version parsing logic I figured we could add a check here to warn folks, we could also throw here, but the driver will crash elsewhere anyway "bson is not a constructor" is likely the first error to be hit.
There was a problem hiding this comment.
wouldn't it be more helpful to throw here with the appropriate error instead of letting it fail in a weird way down the line? also, why not put the code doing the check in the if (optionalBSON) block?
There was a problem hiding this comment.
Changed it, and I tested this, it throws at the moment of require('mongodb') so it'll stop scripts right up front.
| if (version.major >= 7) { | ||
| const compressOriginal = snappy.compress; | ||
| const uncompressOriginal = snappy.uncompress; | ||
| snappy.compress = (data, callback) => { |
There was a problem hiding this comment.
Slightly different approach taken, just wrap the functions here, the actual usage site needs no changes now.
| SINGLETON_TASKS.push({ | ||
| name: 'run-custom-csfle-tests', | ||
| tags: ['run-custom-csfle-tests'], | ||
| const oneOffFuncs = [ |
There was a problem hiding this comment.
Ported over the CI changes, I actually took a stab at bringing in the bson-ext CI here too, but it started having too much overhead for this PR, but this sets us up to have easier one-off targets going forward.
| fileData.tasks = (fileData.tasks || []) | ||
| .concat(BASE_TASKS) | ||
| .concat(TASKS) | ||
| .concat(SINGLETON_TASKS); |
There was a problem hiding this comment.
| fileData.tasks = (fileData.tasks || []) | |
| .concat(BASE_TASKS) | |
| .concat(TASKS) | |
| .concat(SINGLETON_TASKS); | |
| fileData.tasks = (fileData.tasks || []).concat(BASE_TASKS, TASKS, SINGLETON_TASKS); |
There was a problem hiding this comment.
done! did it locally along with prettier just to make sure the formatting is right
Not a straight port, see comments