POC for controlled development environment and usage instructions. #165

peterschutt · 2022-06-22T08:52:01Z

To try to clear the path for contributors it would be good to have a specific workflow that we use. We could even use tox inside CI so that there would be virtually no difference between running the tests locally and in CI.

Basic workflow is:

developer ensures they have the versions of python available - there are instructions on how to use pyenv for this in CONTRIBUTING.md
developer uses poetry to install Starlite and required extras, poetry install --extras "test lint dev" (I want to collapse this down to poetry install --extras dev` but I'm having trouble with circular dependencies, which apparently should be supported: https://discuss.python.org/t/pyproject-toml-optional-dependencies-redundancy-aka-dry-extras/8428/5).
everything should work as expected from then on, and we'll have poetry run tox which will run tests across all python versions and linting.

Bobronium · 2022-06-24T02:07:57Z

Instead of extras, you might want to utilize Dependency groups.

Though, it's not available in stable release yet, it's seems like it does exactly what you're trying to achieve here. So maybe it's worth waiting for poetry 1.2.0 (it's now in beta)

peterschutt · 2022-06-24T02:53:27Z

Thanks for input @Bobronium

It seems they are aware that there is a lot of production users of 1.2 beta as it is: python-poetry/poetry#5586 (comment):

While releasing often might be good, right now due to the amount of folks using 1.2.0b1 in production workflows, it might pose a larger issue if we release with known breakages.

So jumping early is one option.

Having said that though, if we abstract the workflow below some Makefile commands, we can chop and change the underlying implementation, hopefully without breaking or changing too much of the top level commands.

The core issue here is that we want tox to install our ~~locked~~ test dependencies which is what using extras allows, and so too would using the dependency groups as you've suggested.

tox doesn't need poetry to install our package to test against, rather it builds the source distribution itself and runs the tests against that. I think this is good as it better emulates what our downstream users are doing when they install the library in their own projects. If I understand correctly, using the dependency groups would require us to have poetry in the tox environment in order to do poetry install --with tests or whatever, correct?

peterschutt · 2022-06-24T06:55:14Z

@Goldziher I needed to change the CI anyway to get the tests passing again.

It runs all the same tests and checks, however, it uses tox as the test runner so it is consistent with using tox locally. It runs pytest, pre-commit and generates the coverage report and uploads to sonar.

This configuration allows running coverage across multiple versions and then combines the reports at the end - this is good for e.g., where I couldn't reach the else branch of if sys.version_info >= (3, 10). So it is configured to run coverage on the highest and lowest versions, and then combine the reports.

Otherwise everything else should be there, maybe just ordered slightly differently. Also, with all the python versions in your local environment you can just run poetry run tox to run all the checks just as they are run in CI.

I haven't got the virtualenv's cached like you have in the current version which probably slows it down a bit. I have to check how that works with tox. Most likely it's possible but I've run out of time for now.

There's still work to do on the PR but if you want to provide any feedback on the CI then I'm all ears.

FYI the CI is heavily influenced by attrs: https://github.com/python-attrs/attrs/blob/main/.github/workflows/main.yml

Bobronium · 2022-06-25T20:18:26Z

Thinking about this more, I'd wouch against turning dev-requirements into extras, for one reason: I see extras as something additional when you install the package, not when you develop a package. I haven't find such strict division in https://peps.python.org/pep-0508/#extras, but I feel like this interpretation is pretty common.

Dev requirements are not (and IMO shouldn't be) considered during resolving dependencies when installing wheels.

I feel like I'm communicating my point poorly, but I guess I don't have enough brain capacity to do it differently right now.

Goldziher · 2022-06-25T20:27:40Z

Thinking about this more, I'd wouch against turning dev-requirements into extras, for one reason: I see extras as something additional when you install the package, not when you develop a package. I haven't find such strict division in https://peps.python.org/pep-0508/#extras, but I feel like this interpretation is pretty common.

Dev requirements are not (and IMO shouldn't be) considered during resolving dependencies when installing wheels.

I feel like I'm communicating my point poorly, but I guess I don't have enough brain capacity to do it differently right now.

I tend to agree

peterschutt · 2022-06-25T21:09:34Z

From our own lockfile:

test = ["coverage[toml] (>=4.5)", "hypothesis (>=4.0)", "pytest (>=7.0)", "pytest-mock (>=3.6.1)", "trustme", "contextlib2", "uvloop (<0.15)", "mock (>=4)", "uvloop (>=0.15)"]
tests = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "six", "mypy", "pytest-mypy-plugins", "zope.interface", "cloudpickle"]
tests_no_zope = ["coverage[toml] (>=5.0.2)", "hypothesis", "pympler", "pytest (>=4.3.0)", "six", "mypy", "pytest-mypy-plugins", "cloudpickle"]
test = ["flake8 (==3.7.8)", "hypothesis (==3.55.3)"]
test = ["pytest (>=6)"]
testing = ["covdefaults (>=1.2.0)", "coverage (>=4)", "pytest (>=4)", "pytest-cov", "pytest-timeout (>=1.4.2)"]
testing = ["pytest (>=6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest-cov", "pytest-enabler (>=1.0.1)", "packaging", "pyfakefs", "flufl.flake8", "pytest-perf (>=0.9.2)", "pytest-black (>=0.3.7)", "pytest-mypy (>=0.9.1)", "importlib-resources (>=1.3)"]
testing = ["pytest"]
testing = ["coverage", "pyyaml"]
test = ["appdirs (==1.4.4)", "pytest-cov (>=2.7)", "pytest-mock (>=3.6)", "pytest (>=6)"]
testing = ["pytest", "pytest-benchmark"]
testing = ["argcomplete", "hypothesis (>=3.56)", "mock", "nose", "pygments (>=2.7.2)", "requests", "xmlschema"]
testing = ["coverage (==6.2)", "hypothesis (>=5.7.1)", "flaky (>=3.5.0)", "mypy (==0.931)", "pytest-trio (>=0.7.0)"]
testing = ["fields", "hunter", "process-tests", "six", "pytest-xdist", "virtualenv"]
testing = ["flaky (>=3.4.0)", "freezegun (>=0.3.11)", "pytest (>=4.0.0)", "pytest-cov (>=2.5.1)", "pytest-mock (>=1.10.0)", "pytest-randomly (>=1.0.0)", "psutil (>=5.6.1)", "pathlib2 (>=2.3.3)"]
testing = ["coverage (>=4)", "coverage-enable-subprocess (>=1)", "flaky (>=3)", "pytest (>=4)", "pytest-env (>=0.6.2)", "pytest-freezegun (>=0.4.1)", "pytest-mock (>=2)", "pytest-randomly (>=1)", "pytest-timeout (>=1)", "packaging (>=20.0)"]
testing = ["pytest (>=6)", "pytest-checkdocs (>=2.4)", "pytest-flake8", "pytest-cov", "pytest-enabler (>=1.0.1)", "jaraco.itertools", "func-timeout", "pytest-black (>=0.3.7)", "pytest-mypy (>=0.9.1)"]

The pattern is hardly uncommon - because it solves a real problem. Have a look for yourselves.

peterschutt · 2022-06-26T00:03:40Z

@Bobronium I'm merely trying to point out that the solution I've arrived at here is a functional consequence of trying to centralize our dev dependencies across two environments, poetry and tox. The grep above shows just how common this pattern is in the python ecosystem.

I appreciate your passion for pep-8 and I note your ideological disapproval, but what about the real issues I'm trying to solve here? Any input on how you'd prefer a library config that supports those of us who want to test the library locally across different python versions?

I see extras as something additional when you install the package, not when you develop a package.

I see this differently. I see extras as things that provide utility to your users. We have two classes of users, users that depend on starlite and contributors/developers.

Dev requirements are not (and IMO shouldn't be) considered during resolving dependencies when installing wheels.

But we should be testing our library against an installed distribution, such as what tox does, no? I.e., by having tox install the distribution of our library, independent of poetry, we are closer to ensuring that the library will work for any downstream user that installs us through any method. That's where I'm at with this. I think ultimately it is better for us to have tox involved somewhere along the line. Poetry doesn't install extras from dev-dependencies, and so maybe the answer is to just duplicate the dev deps for tox and leave pyproject.toml as it is.

Again, please don't get offended by my wanting to discuss these things, I'd just like to get this sorted so that I can stop having to rebase and cherrypick my contributions.

Bobronium · 2022-06-26T02:01:30Z

I appreciate your passion for pep-8 and I note your ideological disapproval, but what about the real issues I'm trying to solve here? Any input on how you'd prefer a library config that supports those of us who want to test the library locally across different python versions?

I mentioned another approach for this in #165 (comment).

I see this differently. I see extras as things that provide utility to your users. We have two classes of users, users that depend on starlite and contributors/developers.

But we should be testing our library against an installed distribution, such as what tox does, no? I.e., by having tox install the distribution of our library, independent of poetry, we are closer to ensuring that the library will work for any downstream user that installs us through any method. That's where I'm at with this. I think ultimately it is better for us to have tox involved somewhere along the line.

These are good points that I tend to agree with. Being able to setup project environment with plain pip is certainly a good thing. I can't argue with that.

Poetry doesn't install extras from dev-dependencies, and so maybe the answer is to just duplicate the dev deps for tox and leave pyproject.toml as it is.

I'm not sure where you comming from here, could you elaborate? In any case, word 'duplication' scares me. Dev tools are already have to be duplicated between pre-commit and pyproject.toml, introducing the third place to store them doesn't sound like a good idea to me.

I think that I'm just lost here: dev-dependencies looked like a right thing to have, dependencies groups looked like the solution everyone was looking for. Now I'm not sure whether we need them at all, and why we shouldn't just put everything in extras.

pep-8 is a code styleguide, the pep I mentioned was pep-0508 :)

peterschutt · 2022-06-26T02:19:38Z

I'm not sure where you comming from here, could you elaborate?

Sure. From what I can tell, libs tend to put their dev dependencies into a testing extra or something similar, so that the dev deps and their versions can be controlled in pyproject.toml but shared with tox without the need to install poetry into the virtualenvs that tox builds to test the lib. There's a whole host of plugins to help, but when I look at established projects that are using tox, I don't really see those plugins used and only see a testing extras (i've been using attrs as a guide for a lot of this - these are their extras. My point with the grep output was to try to highlight how often this sort of setup is used.

See:

I mentioned another approach for this in #165 (comment).

This question was in my response:

If I understand correctly, using the dependency groups would require us to have poetry in the tox environment in order to do poetry install --with tests or whatever, correct?

I should have been more concise, but I'm trying to avoid having poetry in the tox envs - at least if I can help it.

pep-8 is a code styleguide, the pep I mentioned was pep-0508 :)

Thanks I did misread that.

dev-dependencies looked like a right thing to have

All of the above is about this really. In order to not duplicate lists of dev dependencies, I'm trying to use package extras as a way to declare them in a scope accessible to both tox and poetry.

dependencies groups looked like the solution everyone was looking for

Do I need poetry installed in the tox virtualenvs to install a dependency group? I.e., are they a packaging standard, or just a poetry thing?

Now I'm not sure whether we need them at all, and why we shouldn't just put everything in extras.

Welcome to my world 😆

Bobronium · 2022-06-26T02:29:13Z

If I understand correctly, using the dependency groups would require us to have poetry in the tox environment in order to do poetry install --with tests or whatever, correct?

Sorry, I completely missed that.

Do I need poetry installed in the tox virtualenvs to install a dependency group? I.e., are they a packaging standard, or just a poetry thing?

To this and above: yes, I think this is the case. That's why I don't think that this is the best solutions anymore.

I have another idea:

We could use groups, and then just lock to requirements.txt via poetry (AFAIR, there's a command for this). But then what's the point really, if we already can achieve this with extars...

peterschutt · 2022-06-26T02:38:35Z

But then what's the point really, if we already can achieve this with extars...

Well I can't see the obvious usecase for the poetry groups specific to this issue, I admit I'll have to spend more time looking at the poetry feature to see how they intend them to be used.

Not using extras, but if we were to draw the requirements.txt from the lockfile it wouldn't break the existing CI, for one - but I have got the CI running through tox in this PR, also with a pattern that merges coverage reports across versions which can avoid a whole class of pragma: no cover's.

I appreciate your engagement on this. I'm the only python dev in my shop and so we have "our way" of doing things here (which is very similar to all the above) but I'm trying to be conscious of the way others do things too.

Bobronium · 2022-06-26T23:33:28Z

Ok, here's what I have in mind:

Extras pros:

doesn't require poetry to install

Extras cons:

harder to add/manage dev deps (can't add deps directly into extras, have to edit pyproject.toml manually)
dev dependencies are exposed to wheel dependencies (though it's a common practice)
maybe confusing to users who just want to install starlight and now have to differentiate dev extras from user ones
making starlite more prone to accidental breaking changes/less able to change its dev deps

Deps groups pros:

easier management (can add/remove deps via poetry command)

Deps groups cons:

available only in beta (for now)
poetry export doesn't support groups (for now)
requires poetry to be installed (for now?)

In short, extras are making life easier without poetry, but harder if you would always had poetry.
While dependency groups making it great if you have poetry installed, but impossible (for now) without poetry.

peterschutt · 2022-06-27T04:40:07Z

Ok, here's what I have in mind:

Extras pros:
* doesn't require poetry to install
Extras cons:
* harder to add/manage dev deps (can't add deps directly into extras, have to edit `pyproject.toml` manually)

* dev dependencies are exposed to wheel dependencies (though it's a common practice)

* maybe confusing to users who just want to install `starlight` and now have to differentiate dev extras from user ones

* making `starlite` more prone to accidental breaking changes/less able to change its dev deps
Deps groups pros:
* easier management (can add/remove deps via poetry command)
Deps groups cons:
* available only in beta (for now)

* `poetry export` doesn't support groups (for now)

* requires poetry to be installed (for now?)
In short, extras are making life easier without poetry, but harder if you would always had poetry. While dependency groups making it great if you have poetry installed, but impossible (for now) without poetry.

Auto-generating a requirements.txt from our dev deps is starting to look pretty good from my perspective (subject to trying to actually make it work of course).