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

Add improved Bulk Write API for Java Reactive Driver #1583

Open
wants to merge 31 commits into
base: JAVA-4586_bulk-write
Choose a base branch
from

Conversation

vbabanin
Copy link
Member

  • Created and documented the new Reactive Bulk Write API
  • Enabled unified and prose tests for reactive Bulk Write API

JAVA-5530

- Created and documented the new Reactive Bulk Write API
- Enabled unified and prose tests for reactive Bulk Write API

JAVA-5530
@vbabanin vbabanin self-assigned this Dec 13, 2024
org.junit.jupiter.api.Assumptions.assumeTrue(java.lang.Boolean.parseBoolean(toString()), "BULK-TODO implement")
TODO("BULK-TODO implement")
org.junit.jupiter.api.Assumptions.assumeTrue(
java.lang.Boolean.parseBoolean(toString()), "BULK-TODO Kotlin implement")
Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the naming to specifically ignore Kotlin tests in the UnifiedTestRunner to ensure that only these tests are skipped on assumption errors until the Kotlin API is implemented.

@vbabanin vbabanin marked this pull request as ready for review December 14, 2024 06:25
@vbabanin vbabanin requested a review from stIncMale December 14, 2024 06:25
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

Reviewed everything except for

  • ClientBulkWriteOperation.java
  • CursorHelper.java
  • AsyncFunctionsAbstractTest.java

The last reviewed commit: 9a7d9ba.

/**
* To execute a batch means:
* <ul>
* <li>execute a `bulkWrite` command, which creates a cursor;</li>
* <li>consume the cursor, which may involve executing `getMore` commands.</li>
* </ul>
*
* @throws MongoException When a {@linkplain ClientBulkWriteException#getError() top-level error} happens.
* @throws MongoException When a {@linkplain ClientBulkWriteException#getCause() top-level error} happens.
Copy link
Member

Choose a reason for hiding this comment

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

Thank you! I missed this when I refactored out ClientBulkWriteException.getError.

JAVA-5530
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

The last reviewed commit is c2d9b07.

vbabanin and others added 2 commits December 16, 2024 23:09
…WriteOperation.java

Co-authored-by: Valentin Kovalenko <[email protected]>
- Remove redundunt checks.
- Rename test methods.

JAVA-5530
@vbabanin vbabanin requested a review from stIncMale December 17, 2024 08:10
Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin vbabanin requested a review from stIncMale December 20, 2024 18:02
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

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

Thank you 👍 Though something is suspicious with the test failures - there seem to be too many of them.

@vbabanin
Copy link
Member Author

vbabanin commented Dec 20, 2024

Thank you for the great suggestions, @stIncMale! The structure of the async code now aligns more closely with the sync code.

Regarding the tests, the Azure tests failed due to network connectivity issues. After rerunning, they are consistently green.

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.

2 participants