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

[SPARK-18377][SQL] warehouse path should be a static conf #15825

Closed
wants to merge 2 commits into from

Conversation

cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

it's weird that every session can set its own warehouse path at runtime, we should forbid it and make it a static conf.

How was this patch tested?

existing tests.

@cloud-fan
Copy link
Contributor Author

cloud-fan commented Nov 9, 2016

CC @yhuai @rxin @srowen @gatorsmile

@SparkQA
Copy link

SparkQA commented Nov 9, 2016

Test build #68390 has finished for PR 15825 at commit 1200e2d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 9, 2016

Test build #68395 has finished for PR 15825 at commit 1200e2d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

// Initialize default database if it doesn't already exist
createDatabase(defaultDbDefinition, ignoreIfExists = true)
formatDatabaseName(defaultName)
}
Copy link
Member

Choose a reason for hiding this comment

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

To the other reviewers: this is moved to SharedState.scala. The default database is created once and shared by all the sessions.

createDatabase(defaultDbDefinition, ignoreIfExists = true)
formatDatabaseName(defaultName)
}
protected var currentDb = DEFAULT_DATABASE
Copy link
Member

Choose a reason for hiding this comment

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

It might be safer if we call formatDatabaseName like what we did before.

protected var currentDb = formatDatabaseName (DEFAULT_DATABASE)

@gatorsmile
Copy link
Member

Regarding the code changes, LGTM except a couple of minor comments.

BTW, this introduces reasonable external behavior changes. Maybe we need to document it in the release of Spark 2.1

@SparkQA
Copy link

SparkQA commented Nov 13, 2016

Test build #68583 has finished for PR 15825 at commit 66874f0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Nov 15, 2016

Test build #68665 has finished for PR 15825 at commit 66874f0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Nov 16, 2016

Merging in master/branch-2.1.

@cloud-fan
Copy link
Contributor Author

I added a releasenotes label to the ticket, so that we won't forget

@asfgit asfgit closed this in 4ac9759 Nov 16, 2016
asfgit pushed a commit that referenced this pull request Nov 16, 2016
## What changes were proposed in this pull request?

it's weird that every session can set its own warehouse path at runtime, we should forbid it and make it a static conf.

## How was this patch tested?

existing tests.

Author: Wenchen Fan <wenchen@databricks.com>

Closes #15825 from cloud-fan/warehouse.

(cherry picked from commit 4ac9759)
Signed-off-by: Reynold Xin <rxin@databricks.com>
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?

it's weird that every session can set its own warehouse path at runtime, we should forbid it and make it a static conf.

## How was this patch tested?

existing tests.

Author: Wenchen Fan <wenchen@databricks.com>

Closes apache#15825 from cloud-fan/warehouse.
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.

4 participants