[SPARK-30689][CORE][FOLLOW-UP] Rename config name of discovery plugin #27689
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
Test build #118888 has finished for PR 27689 at commit
|
tgravescs
approved these changes
Feb 25, 2020
Contributor
tgravescs
left a comment
tgravescs
left a comment
There was a problem hiding this comment.
looks good.
Again, I assume you ran a manual test to make sure config works still?
Contributor
|
oh never mind on the testing, unit tests cover it |
Contributor
|
LGTM2 |
HyukjinKwon
approved these changes
Feb 26, 2020
Member
|
Merged to master and branch-3.0. |
HyukjinKwon
pushed a commit
that referenced
this pull request
Feb 26, 2020
### What changes were proposed in this pull request? Rename config `spark.resources.discovery.plugin` to `spark.resources.discoveryPlugin`. Also, as a side minor change: labeled `ResourceDiscoveryScriptPlugin` as `DeveloperApi` since it's not for end user. ### Why are the changes needed? Discovery plugin doesn't need to reserve the "discovery" namespace here and it's more consistent with the interface name `ResourceDiscoveryPlugin` if we use `discoveryPlugin` instead. ### Does this PR introduce any user-facing change? No, it's newly added in Spark3.0. ### How was this patch tested? Pass Jenkins. Closes #27689 from Ngone51/spark_30689_followup. Authored-by: yi.wu <yi.wu@databricks.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org> (cherry picked from commit e9fd522) Signed-off-by: HyukjinKwon <gurwls223@apache.org>
Member
Author
|
thanks all! |
sjincho
pushed a commit
to sjincho/spark
that referenced
this pull request
Apr 15, 2020
### What changes were proposed in this pull request? Rename config `spark.resources.discovery.plugin` to `spark.resources.discoveryPlugin`. Also, as a side minor change: labeled `ResourceDiscoveryScriptPlugin` as `DeveloperApi` since it's not for end user. ### Why are the changes needed? Discovery plugin doesn't need to reserve the "discovery" namespace here and it's more consistent with the interface name `ResourceDiscoveryPlugin` if we use `discoveryPlugin` instead. ### Does this PR introduce any user-facing change? No, it's newly added in Spark3.0. ### How was this patch tested? Pass Jenkins. Closes apache#27689 from Ngone51/spark_30689_followup. Authored-by: yi.wu <yi.wu@databricks.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>
HyukjinKwon
reviewed
May 18, 2020
| * throws if the resource isn't discovered. | ||
| */ | ||
| @DeveloperApi | ||
| class ResourceDiscoveryScriptPlugin extends ResourceDiscoveryPlugin with Logging { |
Member
There was a problem hiding this comment.
I just happened to double check these modules. Shall we add @since for APIs ResourceDiscoveryScriptPlugin, ResourceInformation, etc. ?
I remember it was targeted to be APIs for Spark 3.0, and we kind of postponed to Spark 3.1 (?) cc @tgravescs - am I correct? Let's add @since for exposed APIs for Spark 3.0 at least.
Contributor
There was a problem hiding this comment.
this is 3.0 and is a separate feature from the stage level scheduling that got postponed to 3.1. We can add the @SInCE and its 3.0
HyukjinKwon
pushed a commit
that referenced
this pull request
May 20, 2020
…overyScriptPlugin/ResourceInformation ### What changes were proposed in this pull request? Added `since 3.0.0` for `ResourceDiscoveryScriptPlugin` and `ResourceInformation`. ### Why are the changes needed? It's required for exposed APIs(#27689 (comment)). ### Does this PR introduce _any_ user-facing change? Yes, they can easily know when does Spark introduces the API. ### How was this patch tested? Pass Jenkins. Closes #28591 from Ngone51/followup-30689. Authored-by: yi.wu <yi.wu@databricks.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>
HyukjinKwon
pushed a commit
that referenced
this pull request
May 20, 2020
…overyScriptPlugin/ResourceInformation ### What changes were proposed in this pull request? Added `since 3.0.0` for `ResourceDiscoveryScriptPlugin` and `ResourceInformation`. ### Why are the changes needed? It's required for exposed APIs(#27689 (comment)). ### Does this PR introduce _any_ user-facing change? Yes, they can easily know when does Spark introduces the API. ### How was this patch tested? Pass Jenkins. Closes #28591 from Ngone51/followup-30689. Authored-by: yi.wu <yi.wu@databricks.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org> (cherry picked from commit 26bc690) Signed-off-by: HyukjinKwon <gurwls223@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Rename config
spark.resources.discovery.plugintospark.resources.discoveryPlugin.Also, as a side minor change: labeled
ResourceDiscoveryScriptPluginasDeveloperApisince it's not for end user.Why are the changes needed?
Discovery plugin doesn't need to reserve the "discovery" namespace here and it's more consistent with the interface name
ResourceDiscoveryPluginif we usediscoveryPlugininstead.Does this PR introduce any user-facing change?
No, it's newly added in Spark3.0.
How was this patch tested?
Pass Jenkins.