Skip to content

Conversation

@maropu
Copy link
Member

@maropu maropu commented Apr 25, 2020

What changes were proposed in this pull request?

This PR intends to fix test code for using lowercases for the GLOBAL_TEMP_DATABASE config in SparkSessionBuilderSuite. The handling of the config is different between branch-3.0+ and branch-2.4. In branch-3.0+, Spark always lowercases a value in the config, so I think we had better always use lowercases for it in the test.

This comes from the @dongjoon-hyun
comment: #28316 (comment)

Why are the changes needed?

To fix the test failure in branch-2.4.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Fixed the test.

@maropu
Copy link
Member Author

maropu commented Apr 25, 2020

cc: @dongjoon-hyun @cloud-fan @yaooqinn

@yaooqinn
Copy link
Member

yaooqinn commented Apr 25, 2020

LGTM. shall we tag SPARK-31234 too? maybe add some comment with the change

@maropu maropu changed the title [SPARK-31532][SQL][FOLLOWUP] Use lowercases for GLOBAL_TEMP_DATABASE config in SparkSessionBuilderSuite [SPARK-31532][SPARK-31234][SQL][FOLLOWUP] Use lowercases for GLOBAL_TEMP_DATABASE config in SparkSessionBuilderSuite Apr 25, 2020
@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Apr 25, 2020

Hi, @maropu .
Sorry. I wasn't clear enough. I meant a follow PR for branch-2.4 only and to change the string in assert. We should keep .config(GLOBAL_TEMP_DATABASE.key, ... part for test coverage.

@maropu
Copy link
Member Author

maropu commented Apr 25, 2020

Ah, I see. I'll close this.

@maropu maropu closed this Apr 25, 2020
@SparkQA
Copy link

SparkQA commented Apr 25, 2020

Test build #121793 has finished for PR 28338 at commit fea2375.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants