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.

Sign up for GitHub

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

Jump to bottom

fix(NODE-6583): upgrade to BSON v6.10.1 to remove internal unbounded type cache #4338

Merged
merged 11 commits into from
Dec 5, 2024
Merged

fix(NODE-6583): upgrade to BSON v6.10.1 to remove internal unbounded type cache #4338

merged 11 commits into from
Dec 5, 2024

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Nov 21, 2024 •
edited
Loading

Description

What is changing?

  • Improve connection error messages
  • Allow the driver path, bson path, and driver settings to be configured from env vars
  • Unskip: ldjsonMultiFileUpload and ldjsonMultiFileExport
  • Update to fixed BSON
Is there new documentation needed for these changes?

No

What is the motivation for this change?

We weren't able to run our parallel benchmarks without this fix.

Release Highlight

BSON upgraded to v6.10.1

See: https://github.com/mongodb/js-bson/releases/tag/v6.10.1

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken changed the title [WIP] chore(NODE-6552): fix performance test issue fix(NODE-6583): update to fixed BSON version Nov 25, 2024
nbbeeken
nbbeeken commented Nov 25, 2024
View reviewed changes
Copy link
Contributor Author

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

This is waiting on mongodb/js-bson#740 to be merged and released.

src/cmap/connect.ts Outdated Show resolved Hide resolved
src/cmap/connect.ts Outdated Show resolved Hide resolved
test/benchmarks/driverBench/common.js Show resolved Hide resolved
test/benchmarks/driverBench/common.js Show resolved Hide resolved
@nbbeeken nbbeeken force-pushed the NODE-6552-fix-perf-tests branch from 48b0a8d to 679a05c Compare November 27, 2024 17:42
@nbbeeken nbbeeken changed the title fix(NODE-6583): update to fixed BSON version chore(NODE-6583): update to fixed BSON version Dec 3, 2024
@nbbeeken nbbeeken marked this pull request as ready for review December 3, 2024 21:17
@W-A-James W-A-James self-assigned this Dec 3, 2024
@W-A-James W-A-James self-requested a review December 3, 2024 22:05
@W-A-James W-A-James added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Dec 3, 2024
@W-A-James W-A-James changed the title chore(NODE-6583): update to fixed BSON version fix(NODE-6583): update to fixed BSON version Dec 3, 2024
W-A-James
W-A-James requested changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

Changes all look good to me, but I'd prefer if we were to keep the BSON version bump and associated restoring of the previously failing benchmarks in this PR and move the network error changes to a separate PR.

@W-A-James W-A-James self-requested a review December 4, 2024 19:15
W-A-James
W-A-James previously approved these changes Dec 4, 2024
View reviewed changes
@W-A-James W-A-James added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Dec 4, 2024
baileympearson
baileympearson previously approved these changes Dec 4, 2024
View reviewed changes
@nbbeeken nbbeeken changed the title fix(NODE-6583): update to fixed BSON version fix(NODE-6583): upgrade to BSON v6.10.1 Dec 4, 2024
@nbbeeken nbbeeken changed the title fix(NODE-6583): upgrade to BSON v6.10.1 fix(NODE-6583): upgrade to BSON v6.10.1 to remove internal unbounded type cache Dec 4, 2024
@nbbeeken nbbeeken dismissed stale reviews from baileympearson and W-A-James via 885bc91 December 4, 2024 20:02
W-A-James
W-A-James approved these changes Dec 4, 2024
View reviewed changes
test/readme.md Outdated Show resolved Hide resolved
test/readme.md Outdated Show resolved Hide resolved
test/readme.md Outdated Show resolved Hide resolved
W-A-James
W-A-James requested changes Dec 4, 2024
View reviewed changes
Copy link
Contributor

@W-A-James W-A-James left a comment

Choose a reason for hiding this comment

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

See comments made previously. Accidentally hit approve before 🤦🏿

@nbbeeken nbbeeken requested a review from W-A-James December 4, 2024 21:15
@W-A-James W-A-James merged commit 249c279 into main Dec 5, 2024
31 checks passed
@W-A-James W-A-James deleted the NODE-6552-fix-perf-tests branch December 5, 2024 00:25
@github-actions github-actions bot mentioned this pull request Dec 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@W-A-James W-A-James W-A-James approved these changes

@baileympearson baileympearson baileympearson left review comments

Assignees

@W-A-James W-A-James

Labels
Team Review Needs review from team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nbbeeken @baileympearson @W-A-James