Use Poetry as a dependency tool #33
Conversation
aalexfvk
commented
Aug 9, 2023
•
aalexfvk
commented
- Use Poetry as a single tool for environment, dependency managment, building packages and publishing to PyPI.
- Use flat source code layout instead of src-layout (like in ch-backup)
- Related refactoring
|
https://github.com/yandex/ch-tools/actions/runs/5810475057 |
| - name: publish to PYPI | ||
| if: ${{ matrix.target.ubuntu == 'latest' && startsWith(github.ref, 'refs/tags/') }} | ||
| continue-on-error: true | ||
| run: make publish |
There was a problem hiding this comment.
Here we get "HTTP Error 400: The name 'ch-tools' is too similar to an existing project. See https://pypi.org/help/#project-name for more information."
It looks like it is needed to come up with new name at least for uploading on PYPI
There are already similar names in PYPI search
Because PYPI ignores .,- and _, etc. in names
There was a problem hiding this comment.
Let's try clickhouse-tools.
There was a problem hiding this comment.
I changed it. I had to change name in pyproject.toml and in several other places.
And for workflow it's working https://pypi.org/project/clickhouse-tools/.
There was a problem hiding this comment.
ch-tools remains as part of different addresses email, repo etc
There was a problem hiding this comment.
Great! clickhouse-tools / ch-tools eventually get available on pypi.
And I think it's OK to not rename everything from ch-tools to clickhouse-tools. At least, do not rush with it.
| if: always() | ||
| with: | ||
| report_paths: 'tests/reports/*.xml' | ||
|
|
||
|
|
||
| push_to_dockerhub: |
There was a problem hiding this comment.
I haven't touched this job. We need to think about what to do with it eventually.
It takes >5 min for each push in the main.
For example, run it by button (allowed only for members of ch-backup team) and/or schedule by cron.
README.md
Outdated
| make integration-tests | ||
|
|
||
| # integration tests (do not rebuild docker images) | ||
| # useful when you didn't change source code | ||
| cd tests; behave -D skip_setup | ||
| make integration-tests BEHAVE_ARGS="-D skip_setup" |
There was a problem hiding this comment.
It would be good to unify naming of targets in ch-tools and ch-backup.
In ch-backup targets are named test-integration and test-unit.
https://github.com/yandex/ch-backup/blob/8737ac84fe13f82ddb7be37e97b5b9a89fc8e18d/Makefile#L58
I have no preference on naming schema. We can rename targets either here or in ch-backup.
Also it makes sense to unify semantic of targets. In ch-backup the target test-integration implies "-D skip_setup". And there is a logic in makefile that avoid docker image rebuild if there are no source code changes.
There was a problem hiding this comment.
Agree. I changed test target names like in ch-backup and added implicit -D skip_setup . As for other changes (manipulation of inetgration testing environments) I'd propose to to add it in a separate PR.
pyproject.toml
Outdated
| name = "ch-tools" | ||
| license = { file = "LICENSE" } | ||
| dynamic = ["version", "description"] | ||
| version = "2.529.131159744" |
There was a problem hiding this comment.
Shouldn't it be set dynamically using value from version.txt?
And target versioning schema, I suppose, should be based on git tags. Probably with the help of https://github.com/pypa/setuptools_scm. Though this is out of scope of the given PR.
There was a problem hiding this comment.
Yes it is updated dynamically here. I accedintally uploaded it after testing bulding deb packages. As for schema and other refinments, I use the current one and it can be changed in the next PRs.
pyproject.toml
Outdated
| types-pyOpenSSL = "*" | ||
| types-python-dateutil = "*" | ||
| types-pyyaml = "*" | ||
| types-requests = "*" | ||
| types-setuptools = "*" | ||
| types-tabulate = "*" |
There was a problem hiding this comment.
types-* packages don't make much sense without mypy. So it's worth to add python = ">=3.10" for types-* packages too. This should allow to use up-to-date versions of pyyaml, boto and click. At least in ch-backup it helps - yandex/ch-backup@8a02cad#diff-2b4945591edfeaa4cf4d3f155e66d4b43d1bda7a55d881d5cf3107f1b05abbbc / yandex/ch-backup#26
aalexfvk
force-pushed
the
MDB-24641_try_poetry_as_dependency_tool
branch
2 times, most recently
from
3cc61b2
to
444bb42
Compare
Makefile
Outdated
| @echo 'Bumping version into Debian package changelog' | ||
| DEBFULLNAME="Yandex LLC" DEBEMAIL="ch[email protected]" dch --force-bad-version --distribution stable -v `cat version.txt` Autobuild | ||
| echo 'Bumping version into Debian package changelog' | ||
| DEBFULLNAME="Yandex LLC" DEBEMAIL="clickhouse[email protected]" dch --force-bad-version --distribution stable -v $$(cat $(VERSION_FILE)) Autobuild |
There was a problem hiding this comment.
Let's keep [email protected] for now. [email protected] is nonexistent email.
There was a problem hiding this comment.
The result of IDE auto-replace.
Thanks for the thorough review.
aalexfvk
force-pushed
the
MDB-24641_try_poetry_as_dependency_tool
branch
from
444bb42
to
ed49a93
Compare
