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

[Improve][Connector-V2][MongoDB] A BsonInt32 will be convert to a long type #7567

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

loustler
Copy link
Contributor

@loustler loustler commented Sep 4, 2024

Purpose of this pull request

BsonInt32 can be changed to a long type by itself, so we need to support it.

https://github.com/mongodb/mongo-java-driver/blob/525c465df46f791684fccef0a1ba49f72cbba026/bson/src/main/org/bson/BsonInt32.java#L63-L66

Does this PR introduce any user-facing change?

NO

How was this patch tested?

Package the connector-mongo-2.3.7-2.12.15.jar, place it in a docker image, and complete the MongoDB reading test

Check list

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @loustler

@wuchunfu wuchunfu merged commit adf26c2 into apache:dev Sep 4, 2024
5 checks passed
@loustler loustler deleted the improve/mongo-convert-long branch September 4, 2024 04:37
@Hisoka-X
Copy link
Member

Hisoka-X commented Sep 4, 2024

Hi @loustler , I thinked about it again, is it possible to add a UT to cover the newly added logic?

@loustler
Copy link
Contributor Author

loustler commented Sep 4, 2024

Hi @loustler , I thinked about it again, is it possible to add a UT to cover the newly added logic?

Hi @Hisoka-X

What does a UT means? What a UT stand for?

You mean Unit Test? If it correct, how about a new e2e test case for it?

@Hisoka-X
Copy link
Member

Hisoka-X commented Sep 4, 2024

Hi @loustler , I thinked about it again, is it possible to add a UT to cover the newly added logic?

Hi @Hisoka-X

What does a UT means? What a UT stand for?

You mean Unit Test? If it correct, how about a new e2e test case for it?

Sorry. You are right, unit test. But I think add a e2e test case for this small feature is too heavy. We can add some unit test in https://github.com/apache/seatunnel/tree/e31c9c2a9a0804fd24f3ad16ab28fa4fd7ecc2bb/seatunnel-connectors-v2/connector-mongodb/src/test/java/org/apache/seatunnel/connectors/seatunnel/mongodb .
Just like other connectors did. Please refer


or

@loustler
Copy link
Contributor Author

loustler commented Sep 4, 2024

You're right. It can be too heavy for this case.

I'll added a unit test for it with another PR

@loustler
Copy link
Contributor Author

loustler commented Sep 4, 2024

@Hisoka-X
I just added it in #7579

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

Successfully merging this pull request may close these issues.

3 participants