Skip to content

Conversation

@Satyajitv
Copy link

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)
Default topic first checks if TOPIC_OPTION_KEY is provided,
if TOPIC_OPTION_KEY is provided then
defaulttopic=TOPIC_OPTION_KEY
else TOPIC_OPTION_KEY is not provided then
defaulttopic=PATH_OPTION_KEY

How was this patch tested?

Have tested the code in local, but would start writing tests once the approach is confirmed by @jaceklaskowski, as I am expecting change requests.

PF more details on https://issues.apache.org/jira/browse/SPARK-20597

Please review http://spark.apache.org/contributing.html before opening a pull request.

@holdensmagicalunicorn
Copy link

@Satyajitv, thanks! I am a bot who has found some folks who might be able to help with the review:@tdas, @zsxwing and @cloud-fan

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@mahmoudmahdi24
Copy link

Hello @Satyajitv, please rename the title of this PR properly.
The PR title should be of the form [SPARK-xxxx][COMPONENT] Title, where SPARK-xxxx is the relevant JIRA number, COMPONENT is one of the PR categories shown at spark-prs.appspot.com and Title may be the JIRA’s title or a more specific title describing the PR itself.
Take a look to this helpful document : https://spark.apache.org/contributing.html

val defaultTopic = parameters.get(TOPIC_OPTION_KEY).map(_.trim)
// Picks the defaulttopicname from "path" key, an entry in "parameters" Map,
// if no topic key is present in the "parameters" Map and is provided with key "path".
val defaultTopic = parameters.get(TOPIC_OPTION_KEY) match {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this simpler as something like

val defaultTopic = parameters.getOrElse(TOPIC_OPTION_KEY, parameters.get(PATH_OPTION_KEY)).map(_.trim)

@srowen srowen mentioned this pull request Aug 20, 2018
@asfgit asfgit closed this in b8788b3 Aug 21, 2018
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.

5 participants