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

Remove eachdist #1462

Open
codeboten opened this issue Dec 10, 2020 · 8 comments
Open

Remove eachdist #1462

codeboten opened this issue Dec 10, 2020 · 8 comments
Labels
build & infra Issues related to build & infrastructure. triaged

Comments

@codeboten
Copy link
Contributor

Between tox and eachdist, we end up having to maintain and explain multiple ways of doing the same things. As discussed in the 12/10 SIG meeting, the proposal is to remove our reliance on eachdist from the CI and remove it from contributing.md

@codeboten codeboten added the build & infra Issues related to build & infrastructure. label Dec 10, 2020
@aabmass
Copy link
Member

aabmass commented Dec 10, 2020

I am in favor of removing eachdist 🚀

We can replace the eachdist.py develop functionality mentioned here with tox as well. Tox can create a virtualenv outside of .tox for you (example here), so contributors will just run once tox -e dev to create venv.

@aabmass
Copy link
Member

aabmass commented Dec 10, 2020

Re this part of contributing doc:

You can then run scripts/eachdist.py test to test everything or scripts/eachdist.py lint to lint everything (fixing anything that is auto-fixable).

The intention of eachdist.py test is to be faster than running all of the individual tox envs. You can speed up local tox runs with -p flag to run in parallel. I usually just do tox -f py38-test -pauto

@lzchen
Copy link
Contributor

lzchen commented Dec 10, 2020

+1 for removing as well.

@Oberon00
Copy link
Member

Oberon00 commented Dec 11, 2020

Duplication between eachdist and tox is bad. But note that tox can (and does) call the eachdist.py script to avoid duplication. Also, within the toxfile, many things are duplicated for each distribution, while eachdist can be uses to run the same thing for multiple distributions more easily. For example, all the lines at

python -m pip install -e {toxinidir}/opentelemetry-api[test]
python -m pip install -e {toxinidir}/opentelemetry-sdk[test]
python -m pip install -e {toxinidir}/opentelemetry-instrumentation[test]
python -m pip install -e {toxinidir}/opentelemetry-proto[test]
python -m pip install -e {toxinidir}/tests/util[test]
python -m pip install -e {toxinidir}/instrumentation/opentelemetry-instrumentation-opentracing-shim[test]
python -m pip install -e {toxinidir}/exporter/opentelemetry-exporter-jaeger[test]
python -m pip install -e {toxinidir}/exporter/opentelemetry-exporter-opencensus[test]
python -m pip install -e {toxinidir}/exporter/opentelemetry-exporter-otlp[test]
python -m pip install -e {toxinidir}/exporter/opentelemetry-exporter-prometheus[test]
python -m pip install -e {toxinidir}/exporter/opentelemetry-exporter-zipkin[test]
could most likely be replaced by a single call to something like eachdist.py install -- -editable (not sure about the exact syntax).

As for speed, tox still runs the tests for each version of Python (or is there a way to tell it to only run for a certain version?), so if I have at least as many Python versions than CPU cores, it will be slower.

@ocelotl
Copy link
Contributor

ocelotl commented Dec 11, 2020

+1 for removing eachdist. @Oberon00, tox can be run as tox -e py38-test-core-api/sdk/proto/... in order to use only one version.

@Oberon00
Copy link
Member

Well, that ... is the problem, isn't it? That command line is gonna be quite long. Same for what eachdist.py lint (currently also used in tox) does: It globs around quite a bit to get all the files pylinted (e.g. also samples, setup.pys, etc).

@Oberon00
Copy link
Member

Oberon00 commented Dec 11, 2020

I am no longer really involved in development of opentelemetry-python though, so I have no right to object here -- if you find it easy & fast enough to work with just tox, that is fine. If you end up having lots of private shell scripts and aliases then, maybe there is room for improvement 😃

@github-actions
Copy link

github-actions bot commented Apr 9, 2021

This issue was marked stale due to lack of activity. It will be closed in 30 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build & infra Issues related to build & infrastructure. triaged
Projects
None yet
Development

No branches or pull requests

5 participants