-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Drupal can't use composer install because it requires a Github Auth token on installation #4884
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
Comments
It only asks for it when reaching the API rate limit, as authenticated calls have a much higher rate limit. A better solution would be to improve the prompt to better explain why we ask for it, and to allow to skip the authentication by giving an empty token (i.e. just hitting enter), which would also be explained in the message. |
@stof I think you might be on the right track. It would by default leave some people with a odd mix of dist and source checkouts without an explanation. Could we carry that token through all additional requests during the run and/or expose this behavior as an option? |
"It only asks for it when reaching the API rate limit" "and to allow to skip the authentication by giving an empty token (i.e. just hitting enter), which would also be explained in the message." It does that already, but it prompts on every individual download, so you get into a few seconds for a package, same error message again, press enter, over and over. For me I'd show a warning message but not actually interrupt the downloads. composer install takes long enough that I'm not usually sitting holding my breath in front of the terminal window for it to finish. So I'd rather it took longer and actually happened, than go back and see it got stuck half way through. If it's still running, there's the warning message explaining why it's taking so long. |
well, once you give it a token, it won't ask anymore. |
We do not want to force composer users to create a github account. The install process can throw warnings with bells and whistles and Github advertisements, but it should get the job done without prompting or bothering the user. |
If you run in non-interactive mode ( |
I can see the problem.. having some error output in every run is however not nice either. I wonder if Drupal would have enough weight to have GitHub reconsider the 60/h limit without authentication (it is quite low tbh..). Alternatively I suppose we could indeed remove the prompt even in interactive mode, but then we need even more output to tell people how to configure it if they do create a token. |
@Seldaek I think it's worth a github issue - either to raise the limit or for them to actually put the zip requests through a CDN properly so people aren't hitting the API raw every time. However "We do not want to force composer users to create a github account." 100% agreed with this - this is the reason I reverted the Drupal core commit until it was discussed a bit more, and relying on them to raise their API limit doesn't really help with that dependency. |
At the moment, Composer drupal-update can easily hit the Githubt rate limit. I am using Docker Hub for automated build. The rate limit applies on IP address, so more then often the builds are fails. |
Using interactive promptdoesn't help automated build. Github personal token only works if the automated build if NOT open sourced. If it is open sourced, Github will automatically revoke the public personal token. |
My thoughts exactly. But they probably don't want to do this, to avoid 1) big bandwidth cost, 2) using GitHub to distribute pirated content. |
Well, it depends how you store the token. When using Travis, they have a concept of secure environment variables which means that the token won't be public. |
Using secure environment variable for a public Travis build is broken. That is probably a Travis bug, or maybe it is expected behavior for open source builds on Travis. Edit: I stand corrected per @bartfeenstra's correction. Thank you. |
Gitlab CI has variables to though I haven't looked into if they have the level of "secure" that Travis does. As far as I can tell docker hub does not though but that's probably a limitation of docker build itself. You'll probably want the -n option @Seldaek mentioned if composer is detecting an interactive shell. None of the environmental variable discussion is really pertinent to improving the workflow for users without an API key though which is the core of the issue. |
Secure variables work, but not in pull requests through forks. As a consequence, builds for any such PR will fail, because the token cannot be decrypted. |
@catch56 The reason for using the API for downloads is GitHub guarantee that the endpoint will not change, historically Composer used the download URLs from the website but those have changed overtime. If GitHub stopped rate limiting API download calls that'd really help. |
@bartfeenstra it won't fail. It will fallback to the slower source install (cloning a git repo is slower than downloading a zip archive of the code at a specific commit, as it is much bigger) |
What if we look at this from the other direction: The main issue with downloading sources for vendor dependencies is you're downloading a lot more data, since you get the full history even if you don't need it. What if we allowed for shallow clones, or even defaulted to that? Downloading the latest commit should be in the same neighborhood as downloading a static snapshot, and just one or two commits back is not going to add a great deal of cost. Certainly far less than the full history of a project like Drupal, PHPUnit, or Symfony HttpKernel. (Let's face it, it's not like having the code snapshot of Drupal 4.1 or PHPUnit 2 is at all useful to 99.999999% of people installing Drupal or Symfony today.) That would make "skip this noise, just grab sources" a much cheaper and therefore more viable option. |
@Seldaek #4685 should be fairly safe as it only uses the cache for the initial clone, although it requires Git 2.3 but should be possible to replicate that behaviour for older Git's. gitster/git@d35c802 |
For what it's worth OSX 10.11 is bundled with git 2.3.x these days, and it probably wouldn't be too much of a big deal to strongly recommend using 2.3 or newer for people wanting to work on Drupal core. |
I have no idea. What would you like it to be? I have a call with Jono Bacon next week and I will mention. Absolutely no promises but I'll mention it to him. |
@chx well the main point that would be nice is to get the ability to use the zipball URLs without API limit (or with a higher one.. the exact number probably depends on who is running composer), because really this isn't an API call per se. |
earlier @cs278 mentioned using the non rate limited zips in the past. Could we consider using those, falling back on the api, then falling back on the source? That way if they move again things still "work" until the user updates to a release where composer has fixed the URLs. If the API rate limit message also funnelled people to a useful wiki/help page then I think it could also clear things up for people. "Oh I suddenly saw this because github changed things. No big deal, I can update." |
I came here to say what neclimdul said already. +1 |
That sounds good to me as well trying to use the static zips then falling back to the API @Seldaek would that be an acceptable solution for you too? |
I guess we could do this yes, as long as there's a fallback. It can probably be hooked in after FileDownloader:87 once we have $urls we could go through them and add the direct URLs before the api one if one is found. And I guess for future proofing it, in case one of these fails we probably should stop trying for the other URLs. Just in case github removes support for that composer won't try everything with a timeout/error for every package. |
Sounds like pretty robust logic to me. Good catch on the timeout bit, that probably would get annoying. |
how is this possible, that i hit this in in november? :o
it's a machine with fixed ip. strangely I can wget this file without problems? |
Because you use private repositories maybe? Or just vcs repositories
defined in composer.json overall.. Because that's not covered.
|
nope. None of this is used in this project. But it seems to work / won't work, so maybe it's something api limit related. |
Not needed anymore: composer/composer#4884 (comment)
This is not necessary anymore: composer/composer#4884 (comment)
Not needed anymore: composer/composer#4884 (comment)
Not needed anymore: composer/composer#4884 (comment)
Not needed anymore: composer/composer#4884 (comment)
Not needed anymore: composer/composer#4884 (comment)
- Remove unnecessary environment variable GITHUB_OAUTH_TOKEN (former API limit has been [removed](composer/composer#4884 (comment)) in 2016) - Remove manual doctrine/instantiator downgrade - We support only PHP 7.3 and 7.4 as well as M2 starting from v2.3.3 (former versions reached EOL)
* Update Travis CI config - Remove unnecessary environment variable GITHUB_OAUTH_TOKEN (former API limit has been [removed](composer/composer#4884 (comment)) in 2016) - Remove manual doctrine/instantiator downgrade - We support only PHP 7.3 and 7.4 as well as M2 starting from v2.3.3 (former versions reached EOL) * Fix PHP Fatal Error with PHPUnit setUp() must be compatible with PHPUnit\Framework\TestCase::setUp() – add missing declaration of "void" as return value * Trigger Travis CI * CI: Remove testing for M2 v2.3.3 and v2.3.4 Magento version prior to v2.3.5 depend on xdebug<3, so these tests fail with a "Call to undefined function xdebug_disable()" error. We simply stick to the latest two versions of Magento v2.3.x.
Rate limits have been removed from github: composer/composer#4884 (comment)
Rate limits have been removed from github: composer/composer#4884 (comment)
See also https://www.drupal.org/node/1475510
Problem: when running composer install the user is prompted to input a Github auth token. That is a serious burden for 3000+ Drupal core contributors that will need to run composer install and a significant waste of developer time. Some users do not want to create a Github account.
It seems that composer only prompts for a Github auth token in interactive shells? I have never seen that prompt on Travis CI for PHP projects for example.
Proposed solution: make composer install on interactive shells behave the same as in non interactive ones: always fall back to install from source (git clone) and never prompt the user for a Github token. Users should be able to opt-in for faster downloads with Github auth tokens, but this should never be an interactive usability WTF.
The text was updated successfully, but these errors were encountered: