[SPARK-26554][BUILD] Update `release-util.sh` to avoid GitBox fake 200 headers #23476

dongjoon-hyun · 2019-01-06T23:02:09Z

What changes were proposed in this pull request?

Unlike the previous Apache Git repository, new GitBox repository returns a fake HTTP 200 header instead of 404 Not Found header. This makes release scripts out of order. This PR aims to fix it to handle the html body message instead of the fake HTTP headers. This is a release blocker.

$ curl -s --head --fail "https://gitbox.apache.org/repos/asf?p=spark.git;a=commit;h=v3.0.0"
HTTP/1.1 200 OK
Date: Sun, 06 Jan 2019 22:42:39 GMT
Server: Apache/2.4.18 (Ubuntu)
Vary: Accept-Encoding
Access-Control-Allow-Origin: *
Access-Control-Allow-Methods: POST, GET, OPTIONS
Access-Control-Allow-Headers: X-PINGOTHER
Access-Control-Max-Age: 1728000
Content-Type: text/html; charset=utf-8

BEFORE

$ ./do-release-docker.sh -d /tmp/test -n
Branch [branch-2.4]:
Current branch version is 2.4.1-SNAPSHOT.
Release [2.4.1]:
RC # [1]:
v2.4.1-rc1 already exists. Continue anyway [y/n]?

AFTER

$ ./do-release-docker.sh -d /tmp/test -n
Branch [branch-2.4]:
Current branch version is 2.4.1-SNAPSHOT.
Release [2.4.1]:
RC # [1]:
This is a dry run. Please confirm the ref that will be built for testing.
Ref [v2.4.1-rc1]:

How was this patch tested?

Manual.

…0 headers

dongjoon-hyun · 2019-01-06T23:15:10Z

Hi, @vanzin , @srowen , @cloud-fan .
Could you review this PR?

srowen · 2019-01-06T23:32:52Z

dev/create-release/release-util.sh


 function check_for_tag {
-  curl -s --head --fail "$ASF_REPO_WEBUI;a=commit;h=$1" >/dev/null
+  ! curl -s --fail "$ASF_REPO_WEBUI;a=commit;h=$1" | grep '404 Not Found' > /dev/null


What does the ! do -- make this fail if grep successfully found a 404? does grep -v get the same effect? No need to change it, it's fine, just curious

Thank you for review and approval, @srowen . Yes, right. ! does.
We cannot use grep -v which matches all the other lines doesn't have 404 Not Found. grep only returns 1 when no lines are selected.

Oh right, duh, wasn't thinking it through

felixcheung

perhaps add a comment above this?

dongjoon-hyun · 2019-01-07T01:10:14Z

Sure, @felixcheung .

dongjoon-hyun · 2019-01-07T02:36:49Z

Thank you, @HyukjinKwon .

SparkQA · 2019-01-07T03:34:09Z

Test build #100849 has finished for PR 23476 at commit 241c288.

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

dongjoon-hyun · 2019-01-07T03:58:39Z

The last commit is adding comment-only.
Merged to master/branch-2.4.

…0 headers ## What changes were proposed in this pull request? Unlike the previous Apache Git repository, new GitBox repository returns a fake HTTP 200 header instead of `404 Not Found` header. This makes release scripts out of order. This PR aims to fix it to handle the html body message instead of the fake HTTP headers. This is a release blocker. ```bash $ curl -s --head --fail "https://gitbox.apache.org/repos/asf?p=spark.git;a=commit;h=v3.0.0" HTTP/1.1 200 OK Date: Sun, 06 Jan 2019 22:42:39 GMT Server: Apache/2.4.18 (Ubuntu) Vary: Accept-Encoding Access-Control-Allow-Origin: * Access-Control-Allow-Methods: POST, GET, OPTIONS Access-Control-Allow-Headers: X-PINGOTHER Access-Control-Max-Age: 1728000 Content-Type: text/html; charset=utf-8 ``` **BEFORE** ```bash $ ./do-release-docker.sh -d /tmp/test -n Branch [branch-2.4]: Current branch version is 2.4.1-SNAPSHOT. Release [2.4.1]: RC # [1]: v2.4.1-rc1 already exists. Continue anyway [y/n]? ``` **AFTER** ```bash $ ./do-release-docker.sh -d /tmp/test -n Branch [branch-2.4]: Current branch version is 2.4.1-SNAPSHOT. Release [2.4.1]: RC # [1]: This is a dry run. Please confirm the ref that will be built for testing. Ref [v2.4.1-rc1]: ``` ## How was this patch tested? Manual. Closes #23476 from dongjoon-hyun/SPARK-26554. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit fe039fa) Signed-off-by: Dongjoon Hyun <[email protected]>

cloud-fan · 2019-01-07T04:23:22Z

dev/create-release/release-util.sh

 function check_for_tag {
-  curl -s --head --fail "$ASF_REPO_WEBUI;a=commit;h=$1" >/dev/null
+  # Check HTML body messages instead of header status codes. Apache GitBox returns
+  # a header with `200 OK` status code for both existing and non-existing tag URLs


as we can push commit to github repo directly, do we still have sync issues between github repo and apache repo?

if we don't, maybe we can use the github repo to do the release?

Right. GitHub repo can be used.

$ curl -s --head --fail "https://github.com/apache/spark/releases/tag/v2.4.0" > /dev/null $ echo $? 0 $ curl -s --head --fail "https://github.com/apache/spark/releases/tag/v3.0.0" > /dev/null $ echo $? 22

For me, since last bad experience, I'm pushing to only GitBox. So, I'm not sure whether GitHub to GitBox sync is safe or not in these days.

GitHub to GitBox sync must be safe otherwise we have problems. At least I've updated my local ASF_REPO to github repo and merge PR to github repo directly.

Then, shall I change it back to check header messages against GitHub tag urls? I can do that as a followup PR if you want.

Before proceeding that, let me search some from Apache websites.

Good point on the apache policy concerns. For now I think we only need to use github at this particular place, because gitbox has a mistake about the reader status.

Ya. It was just my confusion. My bad. It looks completely okay.

https://reference.apache.org/committer/github

The ASF has a presence on GitHub at https://github.com/apache

https://gitbox.apache.org/

People that do not wish to utilize GitHub may continue using their ASF credentials to push code to gitbox.apache.org - we do not mandate use of one of the other.

Previously, github was able to be used at read-only pointers like pom.xml's connection. After GitBox setup, it's okay for read/write pointers for all GitHub users.

For non-GitHub users (maybe BitBucket or GitLab users), we still use the gitbox URL at write-access-required pointer like pom.xml's developerConnection.

Yes. So for a SPARK-26554 follow-up PR, I'll focus only this check_for_tag function at this time. Thank you for clarification.

@cloud-fan . I made a followup PR, #23482.

SparkQA · 2019-01-07T05:50:32Z

Test build #100853 has finished for PR 23476 at commit 3178a69.

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

…0 headers ## What changes were proposed in this pull request? Unlike the previous Apache Git repository, new GitBox repository returns a fake HTTP 200 header instead of `404 Not Found` header. This makes release scripts out of order. This PR aims to fix it to handle the html body message instead of the fake HTTP headers. This is a release blocker. ```bash $ curl -s --head --fail "https://gitbox.apache.org/repos/asf?p=spark.git;a=commit;h=v3.0.0" HTTP/1.1 200 OK Date: Sun, 06 Jan 2019 22:42:39 GMT Server: Apache/2.4.18 (Ubuntu) Vary: Accept-Encoding Access-Control-Allow-Origin: * Access-Control-Allow-Methods: POST, GET, OPTIONS Access-Control-Allow-Headers: X-PINGOTHER Access-Control-Max-Age: 1728000 Content-Type: text/html; charset=utf-8 ``` **BEFORE** ```bash $ ./do-release-docker.sh -d /tmp/test -n Branch [branch-2.4]: Current branch version is 2.4.1-SNAPSHOT. Release [2.4.1]: RC # [1]: v2.4.1-rc1 already exists. Continue anyway [y/n]? ``` **AFTER** ```bash $ ./do-release-docker.sh -d /tmp/test -n Branch [branch-2.4]: Current branch version is 2.4.1-SNAPSHOT. Release [2.4.1]: RC # [1]: This is a dry run. Please confirm the ref that will be built for testing. Ref [v2.4.1-rc1]: ``` ## How was this patch tested? Manual. Closes apache#23476 from dongjoon-hyun/SPARK-26554. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

…0 headers ## What changes were proposed in this pull request? Unlike the previous Apache Git repository, new GitBox repository returns a fake HTTP 200 header instead of `404 Not Found` header. This makes release scripts out of order. This PR aims to fix it to handle the html body message instead of the fake HTTP headers. This is a release blocker. ```bash $ curl -s --head --fail "https://gitbox.apache.org/repos/asf?p=spark.git;a=commit;h=v3.0.0" HTTP/1.1 200 OK Date: Sun, 06 Jan 2019 22:42:39 GMT Server: Apache/2.4.18 (Ubuntu) Vary: Accept-Encoding Access-Control-Allow-Origin: * Access-Control-Allow-Methods: POST, GET, OPTIONS Access-Control-Allow-Headers: X-PINGOTHER Access-Control-Max-Age: 1728000 Content-Type: text/html; charset=utf-8 ``` **BEFORE** ```bash $ ./do-release-docker.sh -d /tmp/test -n Branch [branch-2.4]: Current branch version is 2.4.1-SNAPSHOT. Release [2.4.1]: RC # [1]: v2.4.1-rc1 already exists. Continue anyway [y/n]? ``` **AFTER** ```bash $ ./do-release-docker.sh -d /tmp/test -n Branch [branch-2.4]: Current branch version is 2.4.1-SNAPSHOT. Release [2.4.1]: RC # [1]: This is a dry run. Please confirm the ref that will be built for testing. Ref [v2.4.1-rc1]: ``` ## How was this patch tested? Manual. Closes apache#23476 from dongjoon-hyun/SPARK-26554. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit fe039fa) Signed-off-by: Dongjoon Hyun <[email protected]>

[SPARK-26554][BUILD] Update release-util.sh to avoid GitBox fake 20…

241c288

…0 headers

srowen approved these changes Jan 6, 2019

View reviewed changes

felixcheung approved these changes Jan 7, 2019

View reviewed changes

Add comments

3178a69

HyukjinKwon approved these changes Jan 7, 2019

View reviewed changes

asfgit closed this in fe039fa Jan 7, 2019

dongjoon-hyun deleted the SPARK-26554 branch January 7, 2019 04:05

cloud-fan reviewed Jan 7, 2019

View reviewed changes

cloud-fan mentioned this pull request Jan 7, 2019

[SPARK-26554][BUILD][FOLLOWUP] Use GitHub instead of GitBox to check HEADER #23482

Closed

[SPARK-26554][BUILD] Update release-util.sh to avoid GitBox fake 200 headers #23476

[SPARK-26554][BUILD] Update release-util.sh to avoid GitBox fake 200 headers #23476

Uh oh!

Conversation

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

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Jan 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felixcheung left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Jan 7, 2019

Uh oh!

dongjoon-hyun commented Jan 7, 2019

Uh oh!

SparkQA commented Jan 7, 2019

Uh oh!

dongjoon-hyun commented Jan 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Jan 7, 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!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[SPARK-26554][BUILD] Update `release-util.sh` to avoid GitBox fake 200 headers #23476

[SPARK-26554][BUILD] Update `release-util.sh` to avoid GitBox fake 200 headers #23476

dongjoon-hyun commented Jan 6, 2019 •

edited

Loading

dongjoon-hyun Jan 7, 2019 •

edited

Loading