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

ddev update check needs to reset .update regularly #479

Closed
rfay opened this issue Sep 25, 2017 · 5 comments
Closed

ddev update check needs to reset .update regularly #479

rfay opened this issue Sep 25, 2017 · 5 comments
Assignees

Comments

@rfay
Copy link
Member

rfay commented Sep 25, 2017

What happened (or feature request):

$ ddev ssh
Could not check for updates. This is most often caused by a networking issue.
DEBU[0000] GET https://api.github.com/repos/drud/ddev/releases?page=1: 403 API rate limit exceeded for 184.166.22.147. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.); rate reset in 1m53.045558445s
$ ls -l ~/.ddev/.update
-rw-rw-r--  1 rfay  staff  0 Aug  7 07:48 /Users/rfay/.ddev/.update

What you expected to happen:

I didn't expect to get rate-limited by github.

It seems that ~/.ddev/.update only gets rebuilt when an actual update is detected. As a result, if I keep my ddev up-to-date, the .update will never be updated, and thus the github api limit will happen quite soon, as it gets hit on every ddev command.

Code at https://github.com/drud/ddev/blob/master/cmd/ddev/cmd/root.go#L76

How to reproduce this:

  • Set the date on ~/.ddev/.update back to sometime in the past, then do a few ddev commands.

Version: Please include the output of ddev version, docker version and the project's .ddev/config.yaml.

Anything else do we need to know:

It might be reasonable to also suggest brew upgrade ddev if on OSX, not just suggest the download, not sure.

@beeradb
Copy link
Contributor

beeradb commented Sep 27, 2017

Bug is pretty easy to see if you read the code at https://github.com/drud/ddev/blob/a0ee2e90c0aa388659d721b4b3f316727fba5506/cmd/ddev/cmd/root.go#L64-L81 Bummer.

Should be a pretty easy fix, though. May be worth pushing out a point release for this.

fwiw I've never run into it personally. It's probably more likely for those of us who are using ddev commands very frequently. However, it's serious enough to warrant a priority fix in my opinion.

@rfay rfay removed the hibernate label Sep 27, 2017
@rfay
Copy link
Member Author

rfay commented Sep 27, 2017

Removed the hibernate tag, as this really should get done. Hope that's ok @rickmanelius - also adding it to current sprint.

Fix is to move the err = updatecheck.ResetUpdateTime(updateFile)
to the top of the if timeToCheckForUpdates { block, so that we always reset with a new file when the update time comes along.

Obviously the test has failed us, so the test also needs to be improved.

@rfay
Copy link
Member Author

rfay commented Sep 27, 2017

Assigned to @nmccrory - it will be interesting to see how you can improve the test to catch this situation.

@rfay
Copy link
Member Author

rfay commented Sep 30, 2017

Followup: we need to plan a point release for next week so this doesn't annoy people.

@rickmanelius
Copy link
Contributor

Noted. At some point, we might want to adopt a policy of minor releases at least every 2 weeks as long as there is a commit. Basically a bump of 0.0.1 if it doesn’t warrant a larger milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants