Skip to content

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

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

Closed
dongjoon-hyun wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions dev/create-release/release-util.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ DRY_RUN=${DRY_RUN:-0}
GPG="gpg --no-tty --batch"
ASF_REPO="https://gitbox.apache.org/repos/asf/spark.git"
ASF_REPO_WEBUI="https://gitbox.apache.org/repos/asf?p=spark.git"
ASF_GITHUB_REPO="https://github.com/apache/spark"
Copy link
Member

@HyukjinKwon HyukjinKwon Jan 7, 2019

Choose a reason for hiding this comment

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

Hm, I wonder if I completely agree with it. Mixing gitbox and github could look a bit confusing. The previous ! curl -s --fail "$ASF_REPO_WEBUI;a=commit;h=$1" | grep '404 Not Found' > /dev/null is good enough I guess. I'm okay but let's don't do this if it's not only me feeling in this way.

Copy link
Contributor

@cloud-fan cloud-fan Jan 7, 2019

Choose a reason for hiding this comment

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

It's good enough but it's a hack to me. Using Github repo at this single place is also a hack for now, but according to #23476 (comment) , seems we can switch to Github repo completely in the future.

Copy link
Member

@HyukjinKwon HyukjinKwon Jan 7, 2019

Choose a reason for hiding this comment

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

If we're going to switch to github, I'm okie. Don't block by me on this. Either way is fine to me.

Copy link
Member

@srowen srowen Jan 7, 2019

Choose a reason for hiding this comment

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

Either should be fine. It has just occurred to me though, what if some of us pushed to gitbox and some to github? how would sync resolve it? what rebases on what? I guess it is better to push to one consistently, and I presume most of us (all?) are pushing to github?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Jan 7, 2019

Choose a reason for hiding this comment

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

For now, I don't see a clear LGTM for this PR.
And, at least, I'm currently pushing GitBox only.
Please let me know if the decision is made.

Copy link
Member

@srowen srowen Jan 7, 2019

Choose a reason for hiding this comment

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

Hm, I suppose I think we should consistently push to github. That was the advice I think we sent (?) to committers, and I can update the committer docs on this if any exist. At least we'd have a standard.

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Jan 7, 2019

Choose a reason for hiding this comment

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

@srowen . To give you my background, I followed your email instruction exactly and hit the followings on the same day after I pushed to GitBox first time.

The above situation was the one I mentioned here: #23476 (comment)

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.

That's the reason I've been careful and tried not to cause any sync issue. If everything has no problem, I have no problem to switch back to the standard day~.

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Jan 7, 2019

Choose a reason for hiding this comment

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

BTW, until now, no one gave approval to this. If we prefer to GitBox, could you guys give +1 for this PR, too?

Copy link
Member

@srowen srowen Jan 7, 2019

Choose a reason for hiding this comment

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

That sync has been resolved, right? or at least, we do need that to work going forward, rather than work around it. Is it a blocker right now?

Overall I think we should standardize on github.com remotes over gitbox, so I think this could change again to use all the github.com remotes. Is that OK with you? that would be OK with me.


function error {
echo "$*"
Expand Down Expand Up @@ -73,9 +74,7 @@ function fcreate_secure {
}

function check_for_tag {
# 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
! curl -s --fail "$ASF_REPO_WEBUI;a=commit;h=$1" | grep '404 Not Found' > /dev/null
curl -s --head --fail "$ASF_GITHUB_REPO/releases/tag/$1" > /dev/null
}

function get_release_info {
Expand Down