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

Hardcode download URLs #5

Merged
merged 1 commit into from May 9, 2018

Conversation

radar
Copy link
Contributor

@radar radar commented Apr 21, 2018

When working on cucumber/cucumber-rails#378, we would sometimes see that the builds would be unable to connect to the Geckodriver. On further investigation, this seems to be caused by the Geckodriver not being downloaded in the first place. I think that this is due to GitHub's API rate limiting certain build machines from accessing their API.

A better approach would be to hardcode the URLs for the assets -- since they're easily predicatable -- and then to bump this gem whenever a new release comes out.

To ensure that the URLs stay valid, I've added additional tests using the http gem that at least ensures that the URLs are reachable across all platforms. This should ensure that this gem stays robust into the future.

When working on cucumber/cucumber-rails#378, we would sometimes see that the builds would be unable to connect to the Geckodriver. On further investigation, this seems to be caused by the Geckodriver not being downloaded in the first place. I think that this is due to GitHub's API rate limiting certain build machines from accessing their API.

A better approach would be to hardcode the URLs for the assets -- since they're easily predicatable -- and then to bump this gem whenever a new release comes out.
@olleolleolle
Copy link

A thoughtful and robust fix, @radar. Thanks for digging into this!

@xtrasimplicity
Copy link

@DevicoSolutions, would it be possible to have this merged? It would be helpful on a number of repositories I've been working on. :)

@dmitryreznik
Copy link
Member

@xtrasimplicity Sure thing. Will merge it today or tomorrow.

@dmitryreznik dmitryreznik merged commit 926dd67 into DevicoSolutions:master May 9, 2018
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

4 participants