Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jan 7, 2019

What changes were proposed in this pull request?

This PR uses GitHub repository instead of GitBox because GitHub repo returns HTTP header status correctly.

How was this patch tested?

Manual.

$ ./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]:

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

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

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

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

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

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

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

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

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

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.

@SparkQA
Copy link

SparkQA commented Jan 7, 2019

Test build #100871 has finished for PR 23482 at commit d7a048b.

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

@cloud-fan
Copy link
Contributor

LGTM for this change. In the followup we can completely switch to github, e.g. update the committer doc, update other release scripts, etc.

@HyukjinKwon
Copy link
Member

Okay, then it's good. LGTM too

@dongjoon-hyun
Copy link
Member Author

Thank you, @cloud-fan and all.
Merged to master/branch-2.4.

asfgit pushed a commit that referenced this pull request Jan 8, 2019
…HEADER

## What changes were proposed in this pull request?

This PR uses GitHub repository instead of GitBox because GitHub repo returns HTTP header status correctly.

## How was this patch tested?

Manual.

```
$ ./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]:
```

Closes #23482 from dongjoon-hyun/SPARK-26554-2.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 6f35ede)
Signed-off-by: Dongjoon Hyun <[email protected]>
@asfgit asfgit closed this in 6f35ede Jan 8, 2019
@dongjoon-hyun dongjoon-hyun deleted the SPARK-26554-2 branch January 8, 2019 06:55
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
…HEADER

## What changes were proposed in this pull request?

This PR uses GitHub repository instead of GitBox because GitHub repo returns HTTP header status correctly.

## How was this patch tested?

Manual.

```
$ ./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]:
```

Closes apache#23482 from dongjoon-hyun/SPARK-26554-2.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
…HEADER

## What changes were proposed in this pull request?

This PR uses GitHub repository instead of GitBox because GitHub repo returns HTTP header status correctly.

## How was this patch tested?

Manual.

```
$ ./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]:
```

Closes apache#23482 from dongjoon-hyun/SPARK-26554-2.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 6f35ede)
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
…HEADER

## What changes were proposed in this pull request?

This PR uses GitHub repository instead of GitBox because GitHub repo returns HTTP header status correctly.

## How was this patch tested?

Manual.

```
$ ./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]:
```

Closes apache#23482 from dongjoon-hyun/SPARK-26554-2.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 6f35ede)
Signed-off-by: Dongjoon Hyun <[email protected]>
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