Skip to content

Conversation

rozza
Copy link
Member

@rozza rozza commented Apr 17, 2024

@rozza rozza requested review from a team, jyemin and stIncMale and removed request for a team April 17, 2024 16:45
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.

There are two optional items. Feel free to merge with or without addressing them.


static ConvertJson fromString(final String jsonType) {
if (jsonType.equalsIgnoreCase(TRUE)) {
static ConvertJson fromString(final String operationType) {
Copy link
Member

Choose a reason for hiding this comment

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

[optional]
Not that I understand how the previous name "jsonType" was a good description for a value of the convertJson option, but neither do I understand why "operationType" is a good description for that. Wouldn't operationType or even just v be more relevant?

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 just used it as its the same name in the constructor. Will update.

WriteConfig writeConfig = MongoConfig.createConfig(CONFIG_MAP).toWriteConfig();
assertEquals(writeConfig.convertJson(), WriteConfig.ConvertJson.FALSE);
assertEquals(
writeConfig.withOption("convertJson", "True").convertJson(), WriteConfig.ConvertJson.ANY);
Copy link
Member

Choose a reason for hiding this comment

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

[optional]
Let's also add

assertEquals(
        writeConfig.withOption("convertJson", "False").convertJson(), WriteConfig.ConvertJson.FALSE);

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@rozza rozza merged commit 382f490 into mongodb:main Apr 18, 2024
@rozza rozza deleted the SPARK-425 branch April 18, 2024 09:50
rozza added a commit to rozza/mongo-spark that referenced this pull request Apr 18, 2024
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