[SPARK-28053][INFRA] Handle a corner case where there is no `Link` header by dongjoon-hyun · Pull Request #24874 · apache/spark

dongjoon-hyun · 2019-06-14T07:16:55Z

What changes were proposed in this pull request?

Currently, github_jira_sync.py assumes that there is Link always. However, it will fail when the number of the open PR is less than 100 (the default paging number). It will not happen in Apache Spark, but we had better fix that because it happens during review process for github_jira_sync.py script.

Traceback (most recent call last):
  File "dev/github_jira_sync.py", line 139, in <module>
    jira_prs = get_jira_prs()
  File "dev/github_jira_sync.py", line 83, in get_jira_prs
    link_header = filter(lambda k: k.startswith("Link"), page.info().headers)[0]
IndexError: list index out of range

How was this patch tested?

Manually check with another repo which has small number of open PRs (< 100).

$ export JIRA_PASSWORD=...
$ export GITHUB_API_BASE='https://api.github.com/repos/your-id/spark'
$ dev/github_jira_sync.py

…ader

dongjoon-hyun · 2019-06-14T07:18:31Z

Hi, @HyukjinKwon .
Could you review this PR? This will be needed for the review process of #24871 .

HyukjinKwon · 2019-06-14T07:28:20Z

        # Check if there is another page
-        link_header = filter(lambda k: k.startswith("Link"), page.info().headers)[0]
-        if "next" not in link_header:
+        link_headers = filter(lambda k: k.startswith("Link"), page.info().headers)


Just wanted to leave a note that filter returns a generator from Python 3... in fact this whole script should be fixed anyway as of SPARK-27889 separately.

>>> filter(lambda x: x, ['a'])[0] Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: 'filter' object is not subscriptable >>> list(filter(lambda x: x, ['a']))[0] 'a'

HyukjinKwon · 2019-06-14T07:30:33Z

-        link_header = filter(lambda k: k.startswith("Link"), page.info().headers)[0]
-        if "next" not in link_header:
+        link_headers = filter(lambda k: k.startswith("Link"), page.info().headers)
+        if not link_headers or "next" not in link_headers[0]:


not link_headers is an idiomatic to check if the list is empty. I personally encourage people to use explicit len(...) > 0 comparison (and arguably other projects do too more (?)). But I guess it's my preference just for now. It might be fine.

I'll use len(...) > 0 for the next time~

HyukjinKwon · 2019-06-14T07:32:17Z

I don't think this script is tested in our PR builder. linter is enough. I'll merge this one after the linter passes

HyukjinKwon · 2019-06-14T07:33:30Z

Merged to master.

dongjoon-hyun · 2019-06-14T08:03:06Z

Thank you so much for the review and merge, @HyukjinKwon !

SparkQA · 2019-06-14T09:25:16Z

Test build #106508 has finished for PR 24874 at commit bcecc49.

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

## What changes were proposed in this pull request? This PR aims to expose JIRA issue component types at GitHub PRs. ## How was this patch tested? Manual. ``` $ export GITHUB_OAUTH_KEY=... $ export JIRA_PASSWORD=... $ export GITHUB_API_BASE='https://api.github.com/repos/your-id/spark' $ dev/github_jira_sync.py ``` Please note that the existing script will raise the following exceptions if your repo has less than 100 PRs. This will be handled at #24874 . ``` Traceback (most recent call last): File "dev/github_jira_sync.py", line 139, in <module> jira_prs = get_jira_prs() File "dev/github_jira_sync.py", line 83, in get_jira_prs link_header = filter(lambda k: k.startswith("Link"), page.info().headers)[0] IndexError: list index out of range ``` That is beyond the scope of this PR. Closes #24871 from dongjoon-hyun/SPARK-28051. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…ader ## What changes were proposed in this pull request? Currently, `github_jira_sync.py` assumes that there is `Link` always. However, it will fail when the number of the open PR is less than 100 (the default paging number). It will not happen in Apache Spark, but we had better fix that because it happens during review process for `github_jira_sync.py` script. ``` Traceback (most recent call last): File "dev/github_jira_sync.py", line 139, in <module> jira_prs = get_jira_prs() File "dev/github_jira_sync.py", line 83, in get_jira_prs link_header = filter(lambda k: k.startswith("Link"), page.info().headers)[0] IndexError: list index out of range ``` ## How was this patch tested? Manually check with another repo which has small number of open PRs (< 100). ``` $ export JIRA_PASSWORD=... $ export GITHUB_API_BASE='https://api.github.com/repos/your-id/spark' $ dev/github_jira_sync.py ``` Closes apache#24874 from dongjoon-hyun/SPARK-28053. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>

## What changes were proposed in this pull request? This PR aims to expose JIRA issue component types at GitHub PRs. ## How was this patch tested? Manual. ``` $ export GITHUB_OAUTH_KEY=... $ export JIRA_PASSWORD=... $ export GITHUB_API_BASE='https://api.github.com/repos/your-id/spark' $ dev/github_jira_sync.py ``` Please note that the existing script will raise the following exceptions if your repo has less than 100 PRs. This will be handled at apache#24874 . ``` Traceback (most recent call last): File "dev/github_jira_sync.py", line 139, in <module> jira_prs = get_jira_prs() File "dev/github_jira_sync.py", line 83, in get_jira_prs link_header = filter(lambda k: k.startswith("Link"), page.info().headers)[0] IndexError: list index out of range ``` That is beyond the scope of this PR. Closes apache#24871 from dongjoon-hyun/SPARK-28051. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

[SPARK-28053][INFRA] Handle a corner case where there is no Link he…

bcecc49

…ader

dongjoon-hyun mentioned this pull request Jun 14, 2019

[SPARK-28051][INFRA] Exposing JIRA issue component types at GitHub PRs #24871

Closed

HyukjinKwon reviewed Jun 14, 2019

View reviewed changes

HyukjinKwon approved these changes Jun 14, 2019

View reviewed changes

HyukjinKwon closed this in 7533ccc Jun 14, 2019

dongjoon-hyun deleted the SPARK-28053 branch June 14, 2019 08:03

Navigation Menu

Uh oh!

[SPARK-28053][INFRA] Handle a corner case where there is no `Link` header - #24874

[SPARK-28053][INFRA] Handle a corner case where there is no `Link` header#24874
dongjoon-hyun wants to merge 1 commit into
apache:masterfrom
dongjoon-hyun:SPARK-28053

dongjoon-hyun commented Jun 14, 2019

Uh oh!

dongjoon-hyun commented Jun 14, 2019 •

edited

Loading

Uh oh!

HyukjinKwon Jun 14, 2019

Uh oh!

HyukjinKwon Jun 14, 2019 •

edited

Loading

Uh oh!

dongjoon-hyun Jun 14, 2019

Uh oh!

HyukjinKwon commented Jun 14, 2019

Uh oh!

HyukjinKwon commented Jun 14, 2019

Uh oh!

dongjoon-hyun commented Jun 14, 2019

Uh oh!

SparkQA commented Jun 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dongjoon-hyun commented Jun 14, 2019

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyukjinKwon Jun 14, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jun 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jun 14, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Jun 14, 2019

Uh oh!

HyukjinKwon commented Jun 14, 2019

Uh oh!

dongjoon-hyun commented Jun 14, 2019

Uh oh!

SparkQA commented Jun 14, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dongjoon-hyun commented Jun 14, 2019 •

edited

Loading

HyukjinKwon Jun 14, 2019 •

edited

Loading