Automatically download chrome driver. #92
Merged
esirK
merged 4 commits from driver_install into master 6 years ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'driver_install'
Deleting a branch is permanent. It CANNOT be undone. Continue?
Automatically downloads chrome driver for users according to their os platform.
I think we should have a way to dynamically get the latest chrome driver instead of statically hard-coding them.
Yeah, I was thinking that actually. It's not great to have to update the URL every time there's a new release.
I'll try to review this today or tomorrow, thanks for the work!
If Not, Download it."""cwd = os.listdir(os.getcwd())webdriver_regex = re.compile('chromedriver')We could maybe also construct the expected filename (i.e.
chromedriver_platform.zipbased onplatform.system()like you're doing belowI'd prefer not shadowing the builtin
file()constructorI like to pass an exit code when calling
sys.exit(), so e.g.sys.exit(1)just to be more explicit about the error codesjust change this to be a single line
return webdriver.Chrome(executable_path=driver_path, options=options)I'd prefer something like
return "{0}/chromedriver".format(os.getcwd())just for sylistic reasonsmaybe webdrivers should be a namedtuple or something at the top level scope of the module? Then you could do
webdrivers.darwinorwebdrivers.linux, etcMight be good to create our own exception type here but I think it's fine as is
Like I mentioned in my other comment I think using the
.format()method of strings should be the way to do this (feel free to disagree, I just think it's a good convention to follow)pbar.finish()puts(colored.yellow("Downloading Chrome Webdriver"))file_name = chrome_webdriver.split('/')[-1]_, file_name = os.path.split(chrome_webdriver)would be clearer IMOI use
_just to signify that we don't care about the first part of the tuple thatos.path.splitreturns, but you could just as well doos.path.split(chrome_webdriver)[1]to get itAny Ideas on how we can do this?
makes sense.
True. But I will go ahead and create our own Exception file just incase someone needs to add more exceptions in future.
pbar.finish()puts(colored.yellow("Downloading Chrome Webdriver"))file_name = chrome_webdriver.split('/')[-1]IMHO, I think this wouldn't be clearer since we will be making the person reading the code think that
chrome_webdriveris a file system path which is not. I think what we can do is add a comment beforefile_name = chrome_webdriver.split('/')[-1]saying create filename from chrome_webdriver url. What do you think?I totally agree with this. I would prefer
fstrings but I am sure not everyone is using python>=3.6 🙂pbar.finish()puts(colored.yellow("Downloading Chrome Webdriver"))file_name = chrome_webdriver.split('/')[-1]Good point, I didn't realize at first that it was supposed to be a URL and not an actual path being split. A comment would help.
I'm going to test it out as well and make sure it works for me
I can handle it in a separate PR. I have some ideas for how to do it nicely.
Seems to work for me on Arch Linux. I'm going to merge it and then I have some ideas for how to handle versioning it a bit better.
Thanks for the help!
Reviewers
881296cd59.