Conda install dependencies#648
Conversation
That's the only functionality that's left
geckodriver will be on path, as it is installed by conda, so the default webdriver settings will find it. We don't need to test this.
Correct typo in run-on-osx-via-docker.sh
It's a brave new world!
birdsarah
changed the title
|
This looks really good to me but I still need to try it out. Thank you so much for this! As for documenting stuff in the wiki @englehardt and I discussed that we want to move our documentation into a |
|
@nhnt11 do you have the time to try out |
I was going to leave this for a follow-up discussion, but with this PR I would debate the usefullness/appropriateness of “run-on-osx-via-docker.sh”. I have run the install script on OSX on my husband’s machine and been able to tweak a few things, check it all runs. OSX and Linux should now be on equal footing when it comes to developer experience. My caveats in the docs are related to me not having fully explored, for example, xvfb and OSX. With that in mind, I don’t see the point of special casing OSX and docker. If we want general “run with docker” stuff - I think that’s well handled in the “openwpm-crawler” repo. (That’s the gist of my thoughts, pushback welcome) |
|
First of all sorry for being unable to read I read
as
Having never looked into the script I expected it to do a lot more than it currently does. And the actual docker exec is just the same thing as for the linux version, so yeah I agree with you, we don't need it anymore. |
And un-hide it.
Add it to docker. update readme.
|
Should be good to go @vringar. Thanks for testing it out. I have tested windows (via WSL), linux (ubuntu), the docker image, and osx. @englehardt i think you’ll mostly be fine with this. Our biggest discussion point was pinning and I worked hard to try and find a good solution for you, there are detailed notes in the PR doc. There are compromises, but I really think it’s good enough like this. It’s certainly working and we can iterate if needed. |
I read through the readme changes and Comment 0 of this PR. I agree you found the best solution, and I really appreciate that we're able to continue to pin all dependencies + easily update them. Simplifying things by removing the dev/non-dev distinction seems like a worthwhile tradeoff. Thanks for putting in the work to figure everything out and for thoroughly documenting it. r+ from me for the meta-review. I'll leave it to @vringar to do a more thorough pass. |
3 participants
Fixes: #615, #638, #628
This PR will result in the following changes:
Firefox installation will not be managed by conda installation for now. There will need to be a follow-on PR and possible work done in conda packaging. The firefox available via
conda install firefox -c conda-forgeis release, not unbranded. (So #66 and #381 will not be resolved here.)Note on one environment: With this PR we'll move to having only one environment instead of a requirements.txt and a requirements-dev.txt. Conda doesn't have the same tooling for managing dependencies as pip-tools. I'm happy with the
repin.shit's pretty simple. But I can't see a lightweight way to make a pinnedenvironment-dev.yamlthat has the same pinningenvironment.yaml. That same pinning is critical to ensure that the tests we run give the same result as someone who would just be usingenvironment.yaml. That's not to say it isn't possible, we can write a bash script to do it, but I can't see that the extra maintenance and complexity is worth it, the only impact, as far as I can see, is that the conda environment grows a little. To measure this, I ranconda packwith and without the dev dependencies. The packed environments is 295M without dev dependencies and 322M with dev dependencies, a difference of <10%. I have, however, kept the input yaml files seperate so it's still clear what dependencies we have for dev and what for prod.Package management desires / constraints:
After merging this PR:
run-on-osx-via-docker.shscript