-
-
Notifications
You must be signed in to change notification settings - Fork 154
Update project dirs and files: introduce the pyproject.toml file with uv project manager.
#379
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
base: master
Are you sure you want to change the base?
Conversation
pyproject.toml + uv project manager.
|
@DaniilAnichin, @9en9i, @dolfinus, @mayty, @jimenezj8, @xzkostyan , hello. Could you possibly have thorough and multiple looks at this PR? I think it is high time for project refreshment concerning its infrastructure. |
| VERSION = (0, 3, 2) | ||
| __version__ = '.'.join(str(x) for x in VERSION) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, I have not found an easier and cheaper way for uv to use dynamic versioning.
This solution - uv-dynamic-versioning - aims to retrieve data from a VCS.
I tried this solution - setuptools-scm - yet it didn't work (an unexpected version_file param or smth).
So, if you can find a better solution which allows the uv manager to support dynamic versioning in a cheap and an easy way, feel free to coautor and propose changes and even commits.
|
A bit later I'll integrate the uv into the GHA worflow files: https://docs.astral.sh/uv/guides/integration/github/ . |
DaniilAnichin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minors, + as usual a bit iffy about removing version from init, though it also probably is not too important; otherwise lgtm
|
Will finish this PR soon (I hope so). |
This reverts commit 122d43e. modified: clickhouse_sqlalchemy/__init__.py modified: clickhouse_sqlalchemy/alembic/dialect.py modified: clickhouse_sqlalchemy/alembic/operations.py modified: clickhouse_sqlalchemy/alembic/renderers.py modified: clickhouse_sqlalchemy/alembic/toimpl.py modified: clickhouse_sqlalchemy/drivers/asynch/base.py modified: clickhouse_sqlalchemy/drivers/asynch/connector.py modified: clickhouse_sqlalchemy/drivers/base.py modified: clickhouse_sqlalchemy/drivers/compilers/ddlcompiler.py modified: clickhouse_sqlalchemy/drivers/compilers/sqlcompiler.py modified: clickhouse_sqlalchemy/drivers/compilers/typecompiler.py modified: clickhouse_sqlalchemy/drivers/http/base.py modified: clickhouse_sqlalchemy/drivers/http/connector.py modified: clickhouse_sqlalchemy/drivers/http/escaper.py modified: clickhouse_sqlalchemy/drivers/http/exceptions.py modified: clickhouse_sqlalchemy/drivers/http/transport.py modified: clickhouse_sqlalchemy/drivers/http/utils.py modified: clickhouse_sqlalchemy/drivers/native/base.py modified: clickhouse_sqlalchemy/drivers/native/connector.py modified: clickhouse_sqlalchemy/drivers/reflection.py modified: clickhouse_sqlalchemy/drivers/util.py modified: clickhouse_sqlalchemy/engines/__init__.py modified: clickhouse_sqlalchemy/engines/base.py modified: clickhouse_sqlalchemy/engines/mergetree.py modified: clickhouse_sqlalchemy/engines/misc.py modified: clickhouse_sqlalchemy/engines/replicated.py modified: clickhouse_sqlalchemy/engines/util.py modified: clickhouse_sqlalchemy/exceptions.py modified: clickhouse_sqlalchemy/ext/clauses.py modified: clickhouse_sqlalchemy/ext/declarative.py modified: clickhouse_sqlalchemy/orm/__init__.py modified: clickhouse_sqlalchemy/orm/query.py modified: clickhouse_sqlalchemy/sql/__init__.py modified: clickhouse_sqlalchemy/sql/ddl.py modified: clickhouse_sqlalchemy/sql/functions.py modified: clickhouse_sqlalchemy/sql/schema.py modified: clickhouse_sqlalchemy/sql/selectable.py modified: clickhouse_sqlalchemy/types/__init__.py modified: clickhouse_sqlalchemy/types/common.py modified: clickhouse_sqlalchemy/types/ip.py modified: clickhouse_sqlalchemy/types/nested.py
| architecture: x64 | ||
|
|
||
| - name: Install the project | ||
| run: uv sync --python ${{ matrix.python-version }} --locked --all-extras --dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to specify this tedious --python {version} flag because uv respects the ".python-version" file and can consult and activate the version specified in it.
| pip install pytest-timeout | ||
| - name: Run flake8 | ||
| run: flake8 | ||
| uv pip install --python ${{ matrix.python-version }} pytest-timeout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only this dependency is required, maybe move into the "pyproject.toml" and so be it and no need to dance with it here.
| if: ${{ matrix.python-version == '2.7' }} | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: 3.8 | ||
| architecture: x64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, is this necessary? This project does not support Python2.7, see the pyproject.toml and setup.py files.
| def _get_reflected_table(connection, table_name, schema=None): | ||
| # Для Alembic < 1.11 | ||
| if alembic_version < (1, 11): | ||
| from alembic.util.sqla_compat import _reflect_table | ||
|
|
||
| return _reflect_table(connection, table_name, schema) | ||
|
|
||
| # Для Alembic >= 1.11 | ||
| inspector = inspect(connection) | ||
| columns = inspector.get_columns(table_name, schema=schema) | ||
| # Соберите Table объект при необходимости | ||
| return columns | ||
|
|
||
|
|
||
| _alembic_reflect_table = _get_reflected_table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to AI and some researches to fact-check its reasoning.
This module is also ruff formatted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohj, there is already a solution and, I guess, much better one: https://github.com/xzkostyan/clickhouse-sqlalchemy/pull/369/files
| @pytest.mark.skip( | ||
| reason="clickhouse_sqlalchemy.exceptions.DatabaseException: Orig exception: Code: 50. Unknown type JSON" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About this thing - that was just an easier way to make the tests pass.
The tests passed, they were only ruff formatted except this part that I couldn't or just did't want to handle.
The server: docker run --rm -p 9000:9000 -p 8123:8123 -e CLICKHOUSE_DB=test -e CLICKHOUSE_USER=default -e CLICKHOUSE_PASSWORD="Pa5sWor3" --name some-clickhouse-server clickhouse/clickhouse-server
The client: docker run -it --rm --network=container:some-clickhouse-server --entrypoint clickhouse-client clickhouse/clickhouse-server
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason it's failing is that experimental JSON behavior was changed in CH 24.12, requiring additional
set use_json_alias_for_old_object_type = 1. Note that versions prior to 24.12 will fail with this command, so one either needs to check for version, or settle on old/new approach only
pyproject.toml + uv project manager.pyproject.toml file with uv project manager.
|
How I ran tests:
If you need to start a CH client: I had to check the behaviour from the Python script with the asynch>=0.2.2,<0.2.4: import asyncio
from asynch import connect
async def main():
conn = await connect(
host="localhost",
port=9000,
user="default",
password="Pa5sWor3",
database="test",
compress=True,
# client_name='my-python-app'
)
cursor = conn.cursor()
await cursor.execute("SELECT 2 + 2;")
rows = await cursor.fetchall()
for row in rows:
print(row)
await conn.close()
# Запуск асинхронного кода
if __name__ == "__main__":
asyncio.run(main())That is why I introduced password: secure-like and this didn't make the test complicated. Also, this note. |
This reverts commit 48109bb. modified: tests/alembic/test_default_schema_comparators.py modified: tests/config.py modified: tests/drivers/asynch/test_base.py modified: tests/drivers/asynch/test_cursor.py modified: tests/drivers/asynch/test_insert.py modified: tests/drivers/asynch/test_select.py modified: tests/drivers/http/test_cursor.py modified: tests/drivers/http/test_escaping.py modified: tests/drivers/http/test_select.py modified: tests/drivers/http/test_stream.py modified: tests/drivers/http/test_transport.py modified: tests/drivers/http/test_utils.py modified: tests/drivers/native/test_base.py modified: tests/drivers/native/test_cursor.py modified: tests/drivers/native/test_insert.py modified: tests/drivers/native/test_select.py modified: tests/drivers/test_clickhouse_dialect.py modified: tests/drivers/test_util.py modified: tests/engines/test_compilation.py modified: tests/engines/test_parse_columns.py modified: tests/engines/test_reflection.py modified: tests/ext/test_declative.py modified: tests/functions/test_count.py modified: tests/functions/test_extract.py modified: tests/functions/test_has.py modified: tests/functions/test_if.py modified: tests/orm/test_select.py modified: tests/session.py modified: tests/sql/test_case.py modified: tests/sql/test_delete.py modified: tests/sql/test_functions.py modified: tests/sql/test_ilike.py modified: tests/sql/test_insert.py modified: tests/sql/test_is_distinct_from.py modified: tests/sql/test_lambda.py modified: tests/sql/test_limit.py modified: tests/sql/test_regexp_match.py modified: tests/sql/test_schema.py modified: tests/sql/test_selectable.py modified: tests/sql/test_update.py modified: tests/test_compiler.py modified: tests/test_reflection.py modified: tests/testcase.py modified: tests/types/test_boolean.py modified: tests/types/test_date32.py modified: tests/types/test_datetime.py modified: tests/types/test_datetime64.py modified: tests/types/test_int128.py modified: tests/types/test_int256.py modified: tests/types/test_ip.py modified: tests/types/test_numeric.py modified: tests/util.py new file: testsrequire.py
| # - name: Lint | ||
| # run: | | ||
| # uv run --python ${{ matrix.python-version }} ruff check | ||
| # uv run --python ${{ matrix.python-version }} ruff format --check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is large enough and if ruff format all files, it'll get much larger, so my proposal is 1) comment these checks TEMPORARILY and make the pipeline pass and 2) the next PRs must be about only ruff format all files or, maybe, one for source-only files and the following for the tests.
| # Registering `clickhouse` as dialect. | ||
| entry_points={ | ||
| 'sqlalchemy.dialects': dialects | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must not be forgotten.
pyproject.toml
Outdated
| "mypy>=1.4.1", | ||
| "ruff>=0.11.12", | ||
| ] | ||
| test = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there was a test section, albeit empty, it would make sense to me to bring it back & add pytest-timeout there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I am not sure yet, but seems reasonable, however, the "dev" section is good enough and does not look like an anti-pattern.
I'll read about pytest-timeout and its "powers". I think I would like it to not affect test running locally and be available in the CI.
|
Only entry points remain + some review notes to be addressed. UPD: waiting for the #369 to be merged. |
Fixes #345 .
Similar to the #346 and #348 (closed) PRs.
Drags some changes from the #364 .
Prepares the ground for future drop of Python 3.8 ( #356 ) and Py3.9 with Py3.10 as a minimum version.
Enum tests are fixed to reduce warning messages ( partially in the #357 ).
Features are described briefly in the "CHANGELOG.md" file, feel fre to have a look at it, but OK, let's list the here as well:
ruff formatsome files: some tests, docs/conf.py and some source filesVision about the following PRs:
ruff formatting files and then enablingruff checking in the CI;Checklist:
flake8and fix issues. -> done with flake8 and ruff.pytestno tests failed. See https://clickhouse-sqlalchemy.readthedocs.io/en/latest/development.html. -> done aspytest --ignore=tests/types/test_json.pybecause of problems in the Fix tests #357 (won't be fixed in this PR).