-
Notifications
You must be signed in to change notification settings - Fork 28k
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-33240][SQL] Fail fast when fails to instantiate configured v2 session catalog #30147
Conversation
Test build #130259 has finished for PR 30147 at commit
|
retest this, please |
Kubernetes integration test starting |
Kubernetes integration test status success |
Kubernetes integration test starting |
Kubernetes integration test status failure |
Test build #130264 has finished for PR 30147 at commit
|
9d862d0
to
8e3ec8b
Compare
Kubernetes integration test starting |
Kubernetes integration test status success |
Test build #130288 has finished for PR 30147 at commit
|
cc. @cloud-fan @rdblue |
assert(e.getMessage.contains("InvalidCatalogClass")) | ||
} finally { | ||
spark.sessionState.catalogManager.reset() | ||
oldCatalogImpl.foreach { cat => spark.conf.set(V2_SESSION_CATALOG_IMPLEMENTATION.key, cat) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already set/unset in before
and afterEach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. So, we have only one thing (@cloud-fan 's advice) to address.
Thanks, @HeartSaVioR .
Kubernetes integration test starting |
Kubernetes integration test status failure |
thanks, merging to master! |
Thanks for reviewing and merging! |
I'll submit a PR for 3.0 branch as well. |
…session catalog ### What changes were proposed in this pull request? This patch proposes to change the behavior on failing fast when Spark fails to instantiate configured v2 session catalog. ### Why are the changes needed? The Spark behavior is against the intention of the end users - if end users configure session catalog which Spark would fail to initialize, Spark would swallow the error with only logging the error message and silently use the default catalog implementation. This follows the voices on [discussion thread](https://lists.apache.org/thread.html/rdfa22a5ebdc4ac66e2c5c8ff0cd9d750e8a1690cd6fb456d119c2400%40%3Cdev.spark.apache.org%3E) in dev mailing list. ### Does this PR introduce _any_ user-facing change? Yes. After the PR Spark will fail immediately if Spark fails to instantiate configured session catalog. ### How was this patch tested? New UT added. Closes apache#30147 from HeartSaVioR/SPARK-33240. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Test build #130343 has finished for PR 30147 at commit
|
What changes were proposed in this pull request?
This patch proposes to change the behavior on failing fast when Spark fails to instantiate configured v2 session catalog.
Why are the changes needed?
The Spark behavior is against the intention of the end users - if end users configure session catalog which Spark would fail to initialize, Spark would swallow the error with only logging the error message and silently use the default catalog implementation.
This follows the voices on discussion thread in dev mailing list.
Does this PR introduce any user-facing change?
Yes. After the PR Spark will fail immediately if Spark fails to instantiate configured session catalog.
How was this patch tested?
New UT added.