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

Cache Binaries #29

Closed
titusfortner opened this issue Jan 2, 2019 · 7 comments
Closed

Cache Binaries #29

titusfortner opened this issue Jan 2, 2019 · 7 comments
Assignees
Milestone

Comments

@titusfortner
Copy link
Owner

Ideally each session shouldn't need to make a network call if the driver has been recently updated. Alternately if there's a way to programmatically establish the correct driver for the browser version and not unnecessarily check for updates.

@seanlinsley
Copy link

It looks like this gem hooks into Selenium here:

https://github.com/SeleniumHQ/selenium/blob/cb7dd78b0a24214450dd70212168f81c0ad50f48/rb/lib/selenium/webdriver/chrome/driver.rb#L37-L41

With these patches:

module Selenium
module WebDriver
module Chrome
def self.driver_path
@driver_path ||= Webdrivers::Chromedriver.update

So, in order to prevent unnecessary network requests, update needs to be restructured.

def update
unless site_available?
return current_version.nil? ? nil : binary
end
# Newer not specified or latest not found, so use existing
return binary if desired_version.nil? && File.exists?(binary)
# Can't find desired and no existing binary
if desired_version.nil?
msg = "Unable to find the latest version of #{file_name}; try downloading manually from #{base_url} and place in #{install_dir}"
raise StandardError, msg
end
if correct_binary?
Webdrivers.logger.debug "Expected webdriver version found"
return binary
end
remove # Remove outdated exe
download # Fetch latest
end

It seems like this might cover all use cases without requiring network requests after the correct version has been installed:

if correct_binary?
  Webdrivers.logger.debug "Expected webdriver version found"
  binary
elsif desired_version.nil?
  raise "Unable to find the latest version of #{file_name}; try downloading manually from #{base_url} and place in #{install_dir}"
else
  remove
  download
end

@titusfortner
Copy link
Owner Author

#correct_binary is still going to make the remote call though.

It's slightly trickier with the new chromedriver version matching, but I'm thinking a way to globally turn off auto updates probably needs to be supported, as well as checking the timestamp on the file and not making another network call if it is greater than a day (or user configurable period) old.

Haven't had a chance to look at the current open PR yet which will influence what this needs to look like.

@seanlinsley
Copy link

#correct_binary is still going to make the remote call though.

Ah, yeah because desired_version falls back to latest_version.

Checking a timestamp to prevent the network requests could end up introducing bugs if the wait period coincides with a browser release. To work around that you could check the age of both the browser, and the driver.

@titusfortner
Copy link
Owner Author

There's two steps now for chrome. Matching versions and ensuring latest point release. Probably not a big deal to be a day behind on a point release by default, so long as the versions are properly matched

@titusfortner titusfortner added this to the 4.0 milestone Mar 19, 2019
@jrochkind
Copy link

jrochkind commented Mar 27, 2019

See also #4, another gotcha the network access leads to, involving Webmock.

I understand the motivation for live checking for versions on every run in webdrivers. But for those who had been using chromedriver-helper without this feature without problems... I do think it would be nice if webdrivers should as an option provide a feature to work similar to chromedriver-helper, where it can use 'last known at time of release' versions without the network being involved.

I'm also not sure what the implications of the network fetch are on CI runtimes on things like travis or circle-ci, perhaps I want to try to get travis to cache the location that webdrivers is going to be downloading to, hmm.

@kapoorlakshya
Copy link
Collaborator

@jrochkind I just submitted a PR that'll get you what you want - no network being involved. It'll allow you to pin down the desired version and limit the gem from updating the binary or performing any network checks until you:

  1. Change the desired version
  2. Or unset it

Please see this spec for an example: https://github.com/titusfortner/webdrivers/pull/49/files#diff-dbad66b82ddd9a9fdd3e819d2421f77fR46

@titusfortner titusfortner self-assigned this May 5, 2019
@titusfortner titusfortner modified the milestones: 4.0, 3.9 May 5, 2019
@titusfortner titusfortner mentioned this issue May 5, 2019
Closed
@titusfortner
Copy link
Owner Author

implemented with #104

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