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

Refresh updatefile on each check even if we're not updating, fixes #479 #489

Merged
merged 2 commits into from
Sep 30, 2017

Conversation

rfay
Copy link
Member

@rfay rfay commented Sep 28, 2017

The Problem/Issue/Bug:

OP #479: Our update strategy is not working because we only refresh the update file if an update is found. Thus we're regularly running afoul of github's API limit, it even broke a test today. Basically, once the update file gets older than a week, every single invocation of ddev results in a hit on github.

How this PR Solves The Problem:

Instead of refreshing the update file when an update is ready, it does it every time we hit the update interval (and before we even check for an update).

Note: This also does the docker check before the update check, because the docker check is fatal and the update check is not. That lets the update check just return with a warning in some situations.

Manual Testing Instructions:

  • Set the date on ~/.ddev/.update back to sometime more than a week in the past, then do a ddev commands. To set the date, try touch -t 201708200000 ~/.update then use ls -al ~/.ddev/.update to review the results.
  • WIthout this patch, you should end up with a github hit; I'm not sure the easiest way to verify that.
  • With this patch, you should end up with a github hit, but your ~/.ddev/.update will have been refreshed, and the next time you do it you won't have that.

Automated Testing Overview:

There were no existing tests around this code (that I found), and I didn't add any.

Related Issue Link(s):

OP #479

Release/Deployment notes:

Our users start getting impacted by this bug one week after first using the tool, and until an update comes out. One way we can help them is by adding this to an interim release, thus causing the file to be refreshed in buggy versions of ddev. So we should prepare a tag and release.

@rfay rfay self-assigned this Sep 28, 2017
@rfay rfay requested a review from beeradb September 28, 2017 18:48
@rfay rfay force-pushed the 20170928_fix_update_github_timeout branch from 5ecc6c2 to 19559b0 Compare September 28, 2017 21:01
Copy link
Contributor

@tannerjfco tannerjfco left a comment

Choose a reason for hiding this comment

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

This all seems reasonable to me. 👍

@rfay rfay merged commit c860c12 into ddev:master Sep 30, 2017
@rfay rfay deleted the 20170928_fix_update_github_timeout branch September 30, 2017 01:16
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.

None yet

2 participants