scylla-ccm
scylla-ccm copied to clipboard
Introduce Poetry as build system
Overview
- Introduces Poetry as both dependency management and easy building tool.
- First step to solving #555
- Add
main()method toccmscript- Easier to refer to it in poetry
- Could be solved even without it, if I investigated more, but I like main() method better anyway
- Update workflows action versions
- Solves deprecated warnings about node12
- Unnecessary for this Pr, I can extract it to a separate one if needed
Why Poetry
Poetry enables to do both dependency management and building in one package. Setuptools could be used as well, but I find Poetry easier to use and it solves both problems at once. It is a modern build system with nice features like dependency groups (so we dont need multiple requirements files) etc. It also has good IDE support, making installing and managing dependencies much easier than it is today.
How to use it
Main difference between using requirements files or setup.py and Poetry is that poetry by default installs everything into venv and manages that env as well. Local development can be setup with simple poetry install, then you can either use poetry run <your_cmd> for running anything inside the poetry venv or you can simply use poetry shell to get into venv shell.
If you want to also install test dependencies you can use poetry install --with=test instead. Building is done through poetry build and poetry publish and you can change CCM version with poetry version.
More importantly, CCM can be installed with pip install git+... even if you dont have poetry installed, so this change should not break any existing workflows.
TODO
- [ ] Test properly
poetry publish - [ ] Clean up project metadata
- [ ] Migrate
pytest.iniintopyproject.toml - [ ] Add build & publish workflow
Testing
- [x] Running this ccm with dtest pipelines (gating / dtest CI)
- https://jenkins.scylladb.com/view/master/job/scylla-master/job/byo/job/dtest-byo/640/. One test failed, but definitely not due to this PR
- [ ] Driver matrices tests pipelines with this version (python/java)
- https://jenkins.scylladb.com/job/scylla-staging/job/phala/job/python-driver-tests/2/consoleText.
Questions for reviewers
- Do you agree with Poetry usage?
- What should the metadata be? (i.e. author, maintainers, etc)
- Should we lock dependencies?
Testing should cover:
- [ ] running this ccm with dtest pipelines (gating / dtest CI)
- [ ] driver matrices tests pipelines with this version (python/java)
- [ ] running this ccm with dtest pipelines (gating / dtest CI)
https://jenkins.scylladb.com/view/master/job/scylla-master/job/byo/job/dtest-byo/640/. Works with no issues (the one fail is unrelated for sure)
- [ ] driver matrices tests pipelines with this version (python/java)
https://jenkins.scylladb.com/job/scylla-staging/job/phala/job/python-driver-tests/2/consoleText. Not sure if tested
correctly, I was able to force custom ccm through my own pipelines fork, but I still see that it installed CCM (probably from dtest requirements.py) from https://github.com/scylladb/scylla-ccm/archive/master.zip.
Questions for reviewers Do you agree with Poetry usage?
I'm not sure, my personal experience with it isn't that great (we use in Argus for example)
What should the metadata be? (i.e. author, maintainers, etc)
even me myself wasn't very consistent with that: https://github.com/scylladb/python-driver/blob/2af442b0e13b92f7406b887667551b1dcf1b2dad/setup.py#L439C3-L439C27 https://github.com/scylladb/scylla-cqlsh/blob/5e20a4b83654d8f9462400cf67e0bc4d68f3b8cd/setup.cfg#L3
I tend to the first, just naming scylla and that's it same as in scylla itself, we don't name the individuals on the RPM/DEB packages for example
Should we lock dependencies?
no we shouldn't lock, it's a library, and with locking it can complex the user of that library. unless we know of something broken specify, that ccm won't work with, and then not pinning down skip, or up from version.
I'm not sure, my personal experience with it isn't that great (we use in Argus for example)
Would love to hear it, my personal experience is positive, but there are plethora of other solutions we could use
@pehala I'll close this one in favor of #630