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.

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

Only run publish GH action for apache repo, not forked #3500

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

kevinjqliu
Copy link
Contributor

@kevinjqliu kevinjqliu commented Nov 8, 2021

The Publish Snapshot to Maven Github Action has been scheduled automatically in my forked repo and failing due to permission issues.

Log:

* What went wrong:
120
Execution failed for task ':iceberg-spark:publishApachePublicationToMavenRepository'.
121
> Failed to publish publication 'apache' to repository 'maven'
122
   > Could not PUT 'https://repository.apache.org/content/repositories/snapshots/org/apache/iceberg/iceberg-spark/0.13.0-SNAPSHOT/maven-metadata.xml'. Received status code 401 from server: Unauthorized
123

Example GH Action run

The GH Action retries automatically (every day?) and will also generate a failure email report. I had to manually disable it on my forked repo.
This PR limits the GH Action to run only on the apache repo.

References:

@github-actions github-actions bot added the INFRA label Nov 8, 2021
Copy link
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

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

This is a good catch. Thank you @kevinjqliu!

cc @nastra who authored this action. Do you have the need to be able to run this action in your fork (where editing this out and pushing it into your fork's master branch wouldn't be a viable work around)?

EDIT: Also, this might be somewhat urgent if somebody's fork does have permissions (which would be surprising but you never know).

@kevinjqliu
Copy link
Contributor Author

I merged this change to my own fork. Seems like it runs on 0 UTC, will report back tmr

@nastra
Copy link
Contributor

nastra commented Nov 9, 2021

This is a good catch. Thank you @kevinjqliu!

cc @nastra who authored this action. Do you have the need to be able to run this action in your fork (where editing this out and pushing it into your fork's master branch wouldn't be a viable work around)?

EDIT: Also, this might be somewhat urgent if somebody's fork does have permissions (which would be surprising but you never know).

@kbendick no I don't need to have this enabled in my own fork, so let's get this merged. @kevinjqliu thanks for bringing this up.

@kbendick
Copy link
Contributor

kbendick commented Nov 9, 2021

This is a good catch. Thank you @kevinjqliu!
cc @nastra who authored this action. Do you have the need to be able to run this action in your fork (where editing this out and pushing it into your fork's master branch wouldn't be a viable work around)?
EDIT: Also, this might be somewhat urgent if somebody's fork does have permissions (which would be surprising but you never know).

@kbendick no I don't need to have this enabled in my own fork, so let's get this merged. @kevinjqliu thanks for bringing this up.

Agreed let’s get this merged ASAP. Feel free to ask @rymurr to merge this since y’all are up. If he’s not available. I’m OOO for much of the day tomorrow but I’ll ask somebody in me timezone otherwise. This seems pretty important.

We should also consider locking down the permissions of this particular job (and eventually all of them). Take a look at the labeler workflow that I added permissions to a while back @nastra.

@rymurr rymurr merged commit 85662d7 into apache:master Nov 9, 2021
@openinx
Copy link
Member

openinx commented Nov 9, 2021

@rymurr , One small piece of advice: Pls check if the commit message are formatted like Build: xxxx before merge PR (if possible).

@rdblue
Copy link
Contributor

rdblue commented Nov 9, 2021

Thanks, @rymurr for merging! Good to see you again.

@kevinjqliu kevinjqliu deleted the kevinjqliu/forked-repo-gh-action branch November 9, 2021 16:43
Initial-neko pushed a commit to Initial-neko/iceberg that referenced this pull request Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants