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

GODRIVER-2725 Allow setting Encoder and Decoder options on a Client. #1282

Merged
merged 7 commits into from
Jun 21, 2023

Conversation

matthewdale
Copy link
Collaborator

@matthewdale matthewdale commented May 31, 2023

GODRIVER-2725

Summary

  • Expose the new bson.Encoder and bson.Decoder configuration options via a new options.BSONOptions client configuration value.
  • Refactor some of the logic for marshaling values and documents to BSON in the Go driver to make passing the new BSONOptions struct more ergonomic.

Background & Motivation

GODRIVER-2716 added a new API for configuring a bson.Encoder and bson.Decoder that replaces the configuration applied via the bsoncodec package. Users need a way to apply those configurations in mongo.Connect also.

@matthewdale matthewdale force-pushed the godriver2725-set-enc-dec branch 6 times, most recently from cbd1e7b to b424c99 Compare June 8, 2023 00:02
@matthewdale matthewdale force-pushed the godriver2725-set-enc-dec branch 5 times, most recently from 972e3d2 to ce7e1bd Compare June 13, 2023 22:55
@matthewdale matthewdale marked this pull request as ready for review June 13, 2023 22:55
Copy link
Collaborator

@qingyang-hu qingyang-hu left a comment

Choose a reason for hiding this comment

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

LGTM

internal/assert/assertion_mongo.go Outdated Show resolved Hide resolved
internal/assert/assertion_mongo.go Show resolved Hide resolved
internal/assert/assertion_mongo.go Outdated Show resolved Hide resolved
internal/assert/assertion_mongo.go Outdated Show resolved Hide resolved
internal/assert/assertion_mongo.go Outdated Show resolved Hide resolved
mongo/integration/change_stream_test.go Show resolved Hide resolved
mongo/integration/client_test.go Show resolved Hide resolved
mongo/mongo.go Outdated Show resolved Hide resolved
mongo/mongo.go Outdated Show resolved Hide resolved
mongo/single_result_test.go Show resolved Hide resolved
mongo/cursor.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

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

LGTM, nice addition

internal/assert/assertion_mongo_test.go Outdated Show resolved Hide resolved
internal/assert/assertion_mongo_test.go Outdated Show resolved Hide resolved
@matthewdale matthewdale merged commit 41ebbc3 into mongodb:master Jun 21, 2023
@lexxxich
Copy link

lexxxich commented Aug 3, 2023

Setting BSONOptions does nothing because MergeCollectionOptions func ignores this attr. Is this a bug?

@matthewdale
Copy link
Collaborator Author

@lexxxich yes that is a bug, thank you for the report! I've created GODRIVER-2937 to track that bug and the resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants