Hi Eric, you know more about the requirements of the CLI build and the python environment than me, so if you think this is the best way forward I have no objections.
I was just slightly worried because with every little thing that we add to the list of requirements for submitting a patch we will lose a small fraction of potential contributors, and wanted to make sure that this decisions was weighted against this. For example, personally I'd rather receive a bugfix with linting errors than the author keeping it for himself. Best regards, Benno On Fri, Jan 12, 2018 at 12:10 PM, Stephan Erb <stephan....@blue-yonder.com> wrote: > For the Apache Aurora project we solved the need for a virtualenv by using > a wrapper-script that will bootstrap a virtualenv on demand. It only > requires Python to be installed but no other packages. It works flawlessly > on Mac and Linux. Maybe this idea is also useful for Mesos. > > Virtualenv wrapper script: https://github.com/apache/ > aurora/blob/master/build-support/virtualenv > Example usage: https://github.com/apache/aurora/blob/master/build- > support/python/make-pycharm-virtualenv#L47-L48 > > Best regards, > Stephan > > On 11.01.18, 19:55, "Eric Chung" <ech...@uber.com> wrote: > > Hi Benno, > > There are different ways to approach this: > > 1. Ideal case: instead of requiring the `virtualenv` package (e.g. on > debian), require `tox` instead. Since virtualenv is a dependency of > tox, we > will not break any existing code that requires virtualenv. The benefit > of > this is that we will not need a "meta" virtaulenv for installing tox. > Downside is that this might break if the user doesn't have enough > permissions to install packages on the host. > > 2. More pragmatic case: setup a minimal "meta" virtualenv that > installs tox > only. This can be done in the `support` dir and the rest of the code > that > requires tox can call tox from there. Benefit is that this won't > require > any dependency changes and should "just work" without any disruption. > Downside is that we still will need some bash scripts to manage the > "meta" > virtualenv. > > To answer your last question, I don't think this will effect the python > bindings, at least initially. The tests that I aim to run with tox are > mostly CLI-related. In the long term though, it may be worth > considering > using tox to perform all python-related build/test tasks. > > Eric > > On Thu, Jan 11, 2018 at 6:33 AM, Benno Evers <bev...@mesosphere.com> > wrote: > > > Hi Eric, > > > > Do I understand correctly that you want to require all developers to > > install tox so that it can be called from a post-commit hook to > create a > > virtualenv in which it will then install pylint with all its > dependencies > > and use that to lint the changed python files? Would it be possible > to just > > require all developers to install pylint instead? > > > > Also, since you mention that you want to use tox to run unit tests in > > src/python, do you plan to integrate this with the existing mesos > build > > system(s)? E.g., would it respect the python-related configuration > settings > > like `PYTHON`, `PYTHON_VERSION`, `--disable-python`, > > `--disable-python-dependency-install`. Or is this change unrelated > to > > building the python bindings? > > > > Best regards, > > Benno > > > > On Tue, Jan 9, 2018 at 3:29 PM, Armand Grillet < > agril...@mesosphere.io> > > wrote: > > > >> Having distributed tox.ini files and being able to run tests for > >> multiple environments will be helpful to develop the new Mesos CLI > thus > >> I support this change. > >> > >> Requiring developers to install tox is indeed the biggest concern > with > >> this change; however, this process should be straightforward as it > uses pip. > >> > >> 2018-01-09 10:03 GMT+01:00 Kevin Klues <klue...@mesosphere.io>: > >> > >>> I'm the one who asked Eric to send this email. I've been meaning to > >>> comment on it and haven't gotten around to it. I support it. We > just need > >>> to make sure and update our CI appropriately for the new > dependency (and > >>> make devs aware of it). > >>> > >>> > >>> On Tue, Jan 9, 2018 at 4:03 AM Benjamin Mahler <bmah...@apache.org > > > >>> wrote: > >>> > >>>> +armand, benno, kevin > >>>> > >>>> On Fri, Jan 5, 2018 at 12:04 PM, Eric Chung <ech...@uber.com> > wrote: > >>>> > >>>>> Hello mesos devs, > >>>>> > >>>>> I'd like to propose that we replace some of our bash scripts for > >>>>> building > >>>>> ad hoc virtualenvs with tox <https://tox.readthedocs.io/ > en/latest/>, > >>>>> a tool > >>>>> for automating lifecycle management of virtualenvs using > declarative > >>>>> configuration files. > >>>>> > >>>>> Specifically, virtualenvs created for the purpose of linting > >>>>> (support/.virtaulenv) and unit testing (in src/python) can be > managed > >>>>> by > >>>>> tox, which provide the following benefits: > >>>>> > >>>>> 1. Eliminate the need for maintaining shell scripts for managing > >>>>> virtualenvs > >>>>> 2. We will no longer need to install *ALL* dependencies into the > same > >>>>> virtualenv for the purpose of linting -- we can have distributed > >>>>> tox.ini > >>>>> files in wherever python linting is required, and just run tox > there. > >>>>> 3. Easily run tests for multiple environments, e.g. python3 vs > python2. > >>>>> This will make migration to python3 much easier, which we are > facing > >>>>> increasing pressure to address. > >>>>> > >>>>> The biggest concern here would probably the change in > dependencies, > >>>>> since > >>>>> it may seem like we're adding an additional dependency to mesos. > >>>>> However > >>>>> since virtualenv is a dependency of tox, we will not break any > existing > >>>>> dependencies, as requiring tox will automatically require > virtualenv. > >>>>> Otherwise I don't really see any downside in making the switch. > >>>>> > >>>>> Please let me know what you think! > >>>>> > >>>>> Eric > >>>>> > >>>> > >>>> > >> > >> > >> -- > >> Armand Grillet > >> Software Engineer, Mesosphere > >> > > > > > > > > -- > > Benno Evers > > Software Engineer, Mesosphere > > > > > -- Benno Evers Software Engineer, Mesosphere
