GODRIVER-2617 Remove deprecated code #1398

qingyang-hu · 2023-09-20T19:17:44Z

Summary

Remove or un-export deprecated code in Go Driver 2.0.

Background & Motivation

options.MergeClientOptions will be removed in GODRIVER-3001.
GODRIVER-2862 and GODRIVER-2927 are not covered in this PR.

matthewdale · 2023-10-04T18:05:09Z

mongo/collection.go

+	bwo := options.BulkWrite()
+	for _, opt := range opts {
+		if opt == nil {
+			continue
+		}
+		if opt.Comment != nil {
+			bwo.Comment = opt.Comment
+		}


The intent of removing the Merge...Options functions is also to remove the bug-prone logic of merging options structs together. Instead, we want to only allow a single options struct and return an error if the user passes more than one.

E.g.

if len(opts) > 1 { return errors.New("only one BulkWriteOptions can be provided") } bwo := options.BulkWrite() if len(opts) == 1 { bwo = opts[0] } ...

That change will be handled in GODRIVER-2696, so it's fine to keep the options struct merging here. Consider this comment resolved.

matthewdale · 2023-10-04T18:08:12Z

mongo/description/server.go

+	Passive                  bool
+	Primary                  address.Address
+	ReadOnly                 bool
+	ServiceID                *primitive.ObjectID // Only set for servers that are deployed behind a load balancer.
 	SessionTimeoutMinutesPtr *int64


Optional: It's a little awkward to leave a field named ...Ptr. Consider renaming SessionTimeoutMinutesPtr to SessionTimeoutMinutes.

prestonvasquez · 2023-10-05T21:13:31Z

examples/documentation_examples/examples.go

@@ -1971,7 +1971,10 @@ func WithTransactionExample(ctx context.Context) error {
 	defer func() { _ = client.Disconnect(ctx) }()

 	// Prereq: Create collections.
-	wcMajority := writeconcern.New(writeconcern.WMajority(), writeconcern.WTimeout(1*time.Second))
+	wcMajority := &writeconcern.WriteConcern{


Can we use the convenience function in the example, instead of using the "majority" magic string?

wcMajority := writeconcern.Majority() wcMajority.WTimeout = 1 * time.Second

prestonvasquez · 2023-10-05T21:14:41Z

examples/documentation_examples/examples.go

@@ -2552,7 +2555,11 @@ func CausalConsistencyExamples(client *mongo.Client) error {

 	// Use a causally-consistent session to run some operations
 	opts := options.Session().SetDefaultReadConcern(readconcern.Majority()).SetDefaultWriteConcern(
-		writeconcern.New(writeconcern.WMajority(), writeconcern.WTimeout(1000)))
+		&writeconcern.WriteConcern{


Can we use the convenience function in the example, instead of using the "majority" magic string?

wcMajority := writeconcern.Majority() wcMajority.WTimeout = 1000

prestonvasquez · 2023-10-05T21:14:58Z

examples/documentation_examples/examples.go

-		readconcern.Majority()).SetDefaultWriteConcern(writeconcern.New(writeconcern.WMajority(),
-		writeconcern.WTimeout(1000)))
+		readconcern.Majority()).SetDefaultWriteConcern(
+		&writeconcern.WriteConcern{


Can we use the convenience function in the example, instead of using the "majority" magic string?

wcMajority := writeconcern.Majority() wcMajority.WTimeout = 1000

mongo/change_stream.go

prestonvasquez · 2023-10-05T21:22:13Z

mongo/change_stream_test.go

+			description: "empty",
+			input:       []*options.ChangeStreamOptions{},
+			want:        &options.ChangeStreamOptions{},
+		},


Can we include a nil case as well?

prestonvasquez · 2023-10-05T21:44:17Z

x/mongo/driver/operation_test.go

-		wcAck := writeconcern.New(writeconcern.WMajority())
-		wcUnack := writeconcern.New(writeconcern.W(0))
+		wcAck := writeconcern.Majority()
+		wcUnack := &writeconcern.WriteConcern{W: 0}


Suggested change

wcUnack := &writeconcern.WriteConcern{W: 0}

wcUnack := writeconcern.Unacknowledged()

prestonvasquez · 2023-10-05T21:45:29Z

mongo/integration/collection_test.go

@@ -1688,7 +1685,7 @@ func TestCollection(t *testing.T) {
 			assert.Equal(mt, res.UpsertedIDs[3].(string), id3, "expected UpsertedIDs[3] to be %v, got %v", id3, res.UpsertedIDs[3])
 		})
 		unackClientOpts := options.Client().
-			SetWriteConcern(writeconcern.New(writeconcern.W(0)))
+			SetWriteConcern(&writeconcern.WriteConcern{W: 0})


Suggested change

SetWriteConcern(&writeconcern.WriteConcern{W: 0})

SetWriteConcern(writeconcern.Unacknowledged())

prestonvasquez · 2023-10-05T21:45:46Z

mongo/integration/index_view_test.go

@@ -264,7 +263,7 @@ func TestIndexView(t *testing.T) {
 			}
 		})
 		unackClientOpts := options.Client().
-			SetWriteConcern(writeconcern.New(writeconcern.W(0)))
+			SetWriteConcern(&writeconcern.WriteConcern{W: 0})


Suggested change

SetWriteConcern(&writeconcern.WriteConcern{W: 0})

SetWriteConcern(writeconcern.Unacknowledged())

prestonvasquez · 2023-10-05T21:46:06Z

mongo/integration/index_view_test.go

@@ -345,7 +344,7 @@ func TestIndexView(t *testing.T) {
 				Name: indexNames[1],
 			})
 		})
-		wc := writeconcern.New(writeconcern.W(1))
+		wc := &writeconcern.WriteConcern{W: 1}


Suggested change

wc := &writeconcern.WriteConcern{W: 1}

wc := writeconcern.W1()

prestonvasquez · 2023-10-05T21:46:34Z

mongo/integration/sessions_test.go

@@ -107,7 +107,7 @@ func TestSessions(t *testing.T) {
 		})
 	})

-	unackWcOpts := options.Collection().SetWriteConcern(writeconcern.New(writeconcern.W(0)))
+	unackWcOpts := options.Collection().SetWriteConcern(&writeconcern.WriteConcern{W: 0})


Suggested change

unackWcOpts := options.Collection().SetWriteConcern(&writeconcern.WriteConcern{W: 0})

unackWcOpts := options.Collection().SetWriteConcern(writeconcern.Unacknowledged())

matthewdale

Looks good 👍

matthewdale · 2023-10-11T22:55:50Z

mongo/collection.go

+	bwo := options.BulkWrite()
+	for _, opt := range opts {
+		if opt == nil {
+			continue
+		}
+		if opt.Comment != nil {
+			bwo.Comment = opt.Comment
+		}


That change will be handled in GODRIVER-2696, so it's fine to keep the options struct merging here. Consider this comment resolved.

prestonvasquez

LGTM

qingyang-hu added 4 commits September 20, 2023 14:16

GODRIVER-2617 WIP

1b99cfd

GODRIVER-2617 WIP

2265cff

GODRIVER-2617 WIP

b07ff6e

GODRIVER-2617 WIP

0a66171

qingyang-hu force-pushed the godriver2617 branch from 5446a0d to 1b99cfd Compare September 21, 2023 21:39

qingyang-hu added 2 commits September 22, 2023 09:21

GODRIVER-2617 WIP

be47df2

Revert mongo.Connect() changes

5202542

qingyang-hu force-pushed the godriver2617 branch 2 times, most recently from d1127f2 to 3ef44aa Compare September 25, 2023 22:26

WIP

f35a93a

qingyang-hu force-pushed the godriver2617 branch from 3ef44aa to f35a93a Compare September 26, 2023 15:27

WIP

cfd0ffc

qingyang-hu force-pushed the godriver2617 branch from 78b17ff to cfd0ffc Compare September 26, 2023 20:24

GODRIVER-2982 Add PR build tags. (mongodb#1387)

fe99af6

qingyang-hu force-pushed the godriver2617 branch from a6d3aca to fe99af6 Compare September 26, 2023 21:22

qingyang-hu added 3 commits September 27, 2023 11:08

WIP

d1cc8c2

WIP

53a3fbe

WIP

f7daae8

qingyang-hu force-pushed the godriver2617 branch 2 times, most recently from f050dc0 to f3fea53 Compare September 27, 2023 22:00

WIP

3ce688d

qingyang-hu force-pushed the godriver2617 branch 9 times, most recently from 628a603 to 5b1da45 Compare September 28, 2023 19:08

qingyang-hu force-pushed the godriver2617 branch 10 times, most recently from e261dc3 to 23df976 Compare October 2, 2023 17:27

Merge master

26f0a2f

qingyang-hu marked this pull request as ready for review October 2, 2023 19:06

qingyang-hu requested a review from a team as a code owner October 2, 2023 19:06

qingyang-hu requested review from prestonvasquez and removed request for a team October 2, 2023 19:06

qingyang-hu had a problem deploying to api-report October 2, 2023 19:07 — with GitHub Actions Error

qingyang-hu requested a review from matthewdale October 2, 2023 19:07

qingyang-hu had a problem deploying to api-report October 2, 2023 19:07 — with GitHub Actions Error

matthewdale reviewed Oct 4, 2023

View reviewed changes

prestonvasquez requested changes Oct 5, 2023

View reviewed changes

qingyang-hu added 4 commits October 11, 2023 14:46

Remove "Ptr" from field names.

29b292e

Update comments on "Merge...Options" functions.

eb224b5

Use convenience writeconcern functions in code.

19243b7

Add test case.

cb7a81b

matthewdale approved these changes Oct 11, 2023

View reviewed changes

qingyang-hu requested a review from prestonvasquez October 13, 2023 14:40

qingyang-hu had a problem deploying to api-report October 13, 2023 14:40 — with GitHub Actions Error

prestonvasquez approved these changes Oct 13, 2023

View reviewed changes

qingyang-hu merged commit 71f65e3 into mongodb:master Oct 13, 2023
36 of 37 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GODRIVER-2617 Remove deprecated code #1398

GODRIVER-2617 Remove deprecated code #1398

qingyang-hu commented Sep 20, 2023 •

edited

Loading

matthewdale Oct 4, 2023

matthewdale Oct 11, 2023

matthewdale Oct 4, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

prestonvasquez Oct 5, 2023

matthewdale left a comment

matthewdale Oct 11, 2023

prestonvasquez left a comment

	wcUnack := &writeconcern.WriteConcern{W: 0}
	wcUnack := writeconcern.Unacknowledged()

	SetWriteConcern(&writeconcern.WriteConcern{W: 0})
	SetWriteConcern(writeconcern.Unacknowledged())

	wc := &writeconcern.WriteConcern{W: 1}
	wc := writeconcern.W1()

	unackWcOpts := options.Collection().SetWriteConcern(&writeconcern.WriteConcern{W: 0})
	unackWcOpts := options.Collection().SetWriteConcern(writeconcern.Unacknowledged())

GODRIVER-2617 Remove deprecated code #1398

GODRIVER-2617 Remove deprecated code #1398

Conversation

qingyang-hu commented Sep 20, 2023 • edited Loading

Summary

Background & Motivation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewdale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prestonvasquez left a comment

Choose a reason for hiding this comment

qingyang-hu commented Sep 20, 2023 •

edited

Loading