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.

Sign up for GitHub

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

Jump to bottom

[SPARK-23063][K8S] K8s changes for publishing scripts (and a couple of other misses) #20256

Closed
wants to merge 4 commits into from
Closed

[SPARK-23063][K8S] K8s changes for publishing scripts (and a couple of other misses) #20256

wants to merge 4 commits into from

Conversation

foxish
Copy link
Contributor

@foxish foxish commented Jan 13, 2018

What changes were proposed in this pull request?

Including the -Pkubernetes flag in a few places it was missed.

How was this patch tested?

checkstyle, mima through manual tests.

@foxish
Copy link
Contributor Author

foxish commented Jan 13, 2018 •
edited
Loading

cc/ @felixcheung @sameeragarwal @vanzin @rxin @liyinan926

@SparkQA
Copy link

SparkQA commented Jan 13, 2018

Test build #86076 has finished for PR 20256 at commit 801f3ec.

  • This patch fails build dependency tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

felixcheung
felixcheung reviewed Jan 13, 2018
View reviewed changes
dev/create-release/releaseutils.py Outdated
@@ -185,6 +185,7 @@ def get_commits(tag):
"graphx": "GraphX",
"input/output": CORE_COMPONENT,
"java api": "Java API",
"kubernetes": "Kubernetes",
Copy link
Member

Choose a reason for hiding this comment

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

this is for the PR title [foo] - I think [k8s] is more widely used, maybe both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we supply a list/tuple there? I've updated it to K8S, but sometimes folks have written k8s or kubernetes in the PR titles by the looks of it.

Copy link
Member

@felixcheung felixcheung Jan 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

so looks like this is for both commit title and JIRA component field... which isn't quite perfect (for example, not R here, among other things)

but in any case, multiple left values can map to the same right value, it looks like

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, okay, I misread that previously - updated the mapping, and it looks like the script turns things to lower case anyway, so k8s and kubernetes ought to cover everything. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

yes, I think this looks right

@SparkQA
Copy link

SparkQA commented Jan 13, 2018

Test build #86077 has finished for PR 20256 at commit df6f49d.

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

@SparkQA
Copy link

SparkQA commented Jan 13, 2018

Test build #86084 has finished for PR 20256 at commit 73fb21e.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 13, 2018

Test build #86086 has finished for PR 20256 at commit b0a2ead.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@foxish
Copy link
Contributor Author

foxish commented Jan 13, 2018

Can someone trigger a retest please?

@SparkQA
Copy link

SparkQA commented Jan 13, 2018

Test build #4046 has finished for PR 20256 at commit b0a2ead.

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

@sameeragarwal
Copy link
Member

verified changes to the release scripts and a did simple grep check -- both LGTM. Thanks!

@felixcheung
Copy link
Member

thanks! merged to master/2.3

asfgit pushed a commit that referenced this pull request Jan 14, 2018
@foxish
[SPARK-23063][K8S] K8s changes for publishing scripts (and a couple o…
1f4a08b 
…f other misses)

## What changes were proposed in this pull request?

Including the `-Pkubernetes` flag in a few places it was missed.

## How was this patch tested?

checkstyle, mima through manual tests.

Author: foxish <[email protected]>

Closes #20256 from foxish/SPARK-23063.

(cherry picked from commit c3548d1)
Signed-off-by: Felix Cheung <[email protected]>
@asfgit asfgit closed this in c3548d1 Jan 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixcheung felixcheung felixcheung approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@foxish @SparkQA @sameeragarwal @felixcheung