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

Add Scala Client Bulk Write API. #1603

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add Scala Client Bulk Write API. #1603

wants to merge 7 commits into from

Conversation

vbabanin
Copy link
Member

vbabanin
vbabanin commented Jan 13, 2025
View reviewed changes
driver-scala/src/test/scala/org/mongodb/scala/ScalaPackageSpec.scala
@@ -20,7 +20,6 @@ import java.util.concurrent.TimeUnit
import _root_.scala.concurrent.duration.Duration
import com.mongodb.{ MongoCredential => JMongoCredential }
import org.bson.BsonDocumentWrapper
import org.bson.codecs.DocumentCodec
Copy link
Member Author

@vbabanin vbabanin Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The import was unused, therefore it was removed.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

Could you add a ticket so that check catches these or spotless removes them when auto formatting?

Copy link
Member Author

@vbabanin vbabanin Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Done! The ticket is created: JAVA-5765.

@vbabanin vbabanin requested a review from rozza January 13, 2025 20:19
@vbabanin vbabanin marked this pull request as ready for review January 13, 2025 20:19
@vbabanin vbabanin changed the title Add Scala Bulk Write API. Add Scala Client Bulk Write API. Jan 13, 2025
@stIncMale
Copy link
Member

There are still two // TODO-JAVA-5531 in the codebase. I believe, we should address them in this PR.

@vbabanin
Remove TODOs.
d3a6eb7 
JAVA-5531
@vbabanin
Copy link
Member Author

There are still two // TODO-JAVA-5531 in the codebase. I believe, we should address them in this PR.

Good catch! Removed, thanks!

@vbabanin vbabanin self-assigned this Jan 14, 2025
rozza
rozza requested changes Jan 16, 2025
View reviewed changes
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

Looking good - found a couple of nits and added a couple of suggestions for scala check tickets to be filed.

driver-scala/src/main/scala/org/mongodb/scala/model/bulk/package.scala
@@ -0,0 +1,244 @@
package org.mongodb.scala.model
Copy link
Member

Choose a reason for hiding this comment

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

!Missing license header (this should be picked up by checkstyle if not could you open a ticket to ensure that it is in the future).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! Added, thanks!

driver-scala/src/main/scala/org/mongodb/scala/model/bulk/package.scala Outdated
/**
* A model for inserting a document.
*
* @since 5.4
Copy link
Member

Choose a reason for hiding this comment

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

Do we need @since for the methods if the object declares it?

Copy link
Member Author

@vbabanin vbabanin Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Initially, I thought it would be more informative for users inspecting the documentation. However, to align with the usual pattern in the docs, I’ve removed it. Thanks!

driver-scala/src/main/scala/org/mongodb/scala/package.scala Outdated
Comment on lines 238 to 239
* The result of an unsuccessful or partially unsuccessful client-level bulk write operation.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* The result of an unsuccessful or partially unsuccessful client-level bulk write operation.
*/
* The result of an unsuccessful or partially unsuccessful client-level bulk write operation.
* @since 5.4
*/

driver-scala/src/test/scala/org/mongodb/scala/ScalaPackageSpec.scala
@@ -20,7 +20,6 @@ import java.util.concurrent.TimeUnit
import _root_.scala.concurrent.duration.Duration
import com.mongodb.{ MongoCredential => JMongoCredential }
import org.bson.BsonDocumentWrapper
import org.bson.codecs.DocumentCodec
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

Could you add a ticket so that check catches these or spotless removes them when auto formatting?

@vbabanin vbabanin requested a review from rozza January 20, 2025 02:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rozza rozza Awaiting requested review from rozza

Assignees

@vbabanin vbabanin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vbabanin @stIncMale @rozza