Skip to content

Conversation

@RPCMoritz
Copy link
Contributor

@RPCMoritz RPCMoritz commented Jan 26, 2023

I'm doubtful that the final sentence of the paragraph is valid for enterprise users either, as the token would either be valid with github.com for authenticating the download calls, or on the Github Enterprise instance, for authenticating the commit.

This addresses #28

@RPCMoritz RPCMoritz changed the title Add hints for enterprise users regarding rate-limiting/GITHUB_TOKEN #28 Add hints for enterprise users regarding rate-limiting/GITHUB_TOKEN Jan 26, 2023
@axel-op
Copy link
Owner

axel-op commented May 16, 2023

Thank you for the PR! Please ignore the failing checks. I've made slight modifications to your commit, do you approve them?

Also, I'm not sure to understand what you mean by:

the token would either be valid with github.com for authenticating the download calls, or on the Github Enterprise instance, for authenticating the commit

@RPCMoritz
Copy link
Contributor Author

I'm referring to problems arising from working on an on-prem, isolated Github Enterprise instance, and consuming content from Github.com in Actions.

In that case, it is not desirable to use a token valid for Github.com for any authentication with the local Github Enterprise instance, as the two are distinct authenticarion realms. It would be impossible to authenticate commits made on an Enterprise Github instance, with the token required to circumvent rate limiting on github.com.

That's mostly an Enterprise problem though, hence why I didn't forge ahead to change the wording.

Thanks for the nice rewording by the way, changes look good! I wonder whether referring to the problem of (enterprise) proxies also making authorization essentially a prerequisite is worth it - but in general I trust people to extrapolate from the information presented here :)

@axel-op axel-op linked an issue May 17, 2023 that may be closed by this pull request
@axel-op
Copy link
Owner

axel-op commented May 20, 2023

You may be right 🤔 I'll probably disable the commit authentication using this parameter in a future version.

Thank you! Merging this PR right now.

@axel-op axel-op merged commit ccabf63 into axel-op:master May 20, 2023
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.

Probable rate limiting issue for enterprise runners

2 participants