-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-2169] Don't copy appName / basePath everywhere. #1252
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
Conversation
|
Tested on top of #1218 and verified app name looks like what I'd expect from the code. |
|
Can one of the admins verify this patch? |
|
Friendly ping. |
|
Jenkins, test this please. |
Instead of keeping copies in all pages, just reference the values kept in the base SparkUI instance (by making them available via getters).
|
Another friendly ping. |
|
Hey @andrewor14 - can you take a look at this one? seems pretty straightforward, though I'm not super familiar with this code. |
|
test this please |
|
QA tests have started for PR 1252. This patch merges cleanly. |
|
QA results for PR 1252: |
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.
does this fit in the line above?
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.
Not completely.
|
@vanzin Thanks for your PR. These are pretty straightforward changes, and I left a couple of minor comments. Otherwise this LGTM. |
Also some other cleanup.
|
Ping. |
|
Sorry @vanzin I've been caught up with another bug that we're still debugging at the moment. I will look at this and the app ID patch some time tomorrow if not today. |
|
Looks great. Thanks @vanzin |
|
Hi, could we get this merged if there's no more feedback? Thanks! |
|
Jenkins retest this please. LGTM pending tests. |
|
Jenkins, test this please. |
|
QA tests have started for PR 1252 at commit
|
|
QA tests have finished for PR 1252 at commit
|
|
Thanks, I've merged this. |
Instead of keeping copies in all pages, just reference the values kept in the base SparkUI instance (by making them available via getters). Author: Marcelo Vanzin <vanzin@cloudera.com> Closes #1252 from vanzin/SPARK-2169 and squashes the following commits: 4412fc6 [Marcelo Vanzin] Simplify UIUtils.headerSparkPage signature. 4e5d35a [Marcelo Vanzin] [SPARK-2169] Don't copy appName / basePath everywhere.
Instead of keeping copies in all pages, just reference the values kept in the base SparkUI instance (by making them available via getters). Author: Marcelo Vanzin <vanzin@cloudera.com> Closes apache#1252 from vanzin/SPARK-2169 and squashes the following commits: 4412fc6 [Marcelo Vanzin] Simplify UIUtils.headerSparkPage signature. 4e5d35a [Marcelo Vanzin] [SPARK-2169] Don't copy appName / basePath everywhere.
…on (apache#1252) ### What changes were proposed in this pull request? This PR cherry-picks the ordering and distribution during table creation to 3.2. The same syntax is supported in 3.0 and 3.1. ### Why are the changes needed? These changes are needed to define a sort key and distribution in Iceberg tables. ### Does this PR introduce _any_ user-facing change? Yes but the changes won't affect anyone except Iceberg users. ### How was this patch tested? This PR comes with tests.
Instead of keeping copies in all pages, just reference the values
kept in the base SparkUI instance (by making them available via
getters).