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

Drop Python 3.8 #356

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Drop Python 3.8 #356

wants to merge 9 commits into from

Conversation

stankudrow
Copy link

Fixes #355

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Ensure PR doesn't contain untouched code reformatting: spaces, etc.
  • Run flake8 and fix issues.
  • Run pytest no tests failed. See https://clickhouse-sqlalchemy.readthedocs.io/en/latest/development.html.

@stankudrow stankudrow marked this pull request as draft December 22, 2024 17:01
@stankudrow
Copy link
Author

stankudrow commented Dec 22, 2024

The lion's share of the changes derives from pyupgrade --py38-plus ... commands.

@9en9i
Copy link

9en9i commented Feb 5, 2025

Hi, are there any issues with this pull request? Can I help with anything?

@9en9i
Copy link

9en9i commented Feb 5, 2025

At the very least, I see outdated metadata in setup.py, it still specifies Python 3.7.

@9en9i
Copy link

9en9i commented Feb 5, 2025

@xzkostyan What do you think about this pull request? Can we refine it? How? So that it can be merged

@stankudrow
Copy link
Author

Hi, are there any issues with this pull request? Can I help with anything?

Hello.

Yeah, you can help in reviewing or even coathoring (or both). First, could you highlight what needs fixing, please?

@stankudrow stankudrow marked this pull request as ready for review February 5, 2025 16:57
Copy link

@9en9i 9en9i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed all the changes, and it looks good! Just some cosmetic updates thanks to pyupgrade.
The only thing is, I’m not sure that a version dropping support for Python 3.8 should be a patch release.

@stankudrow
Copy link
Author

stankudrow commented Feb 5, 2025

Ah, there are problems to be solved first:

tests/types/test_json.py .F                              [ 98%]
tests/types/test_numeric.py ......                       [100%]

=========================== FAILURES ===========================
____________ JSONTestCaseNative.test_select_insert _____________

self = <clickhouse_sqlalchemy.drivers.native.connector.Cursor object at 0x7b54060c9520>
operation = 'CREATE TABLE test (x JSON) ENGINE = Memory'
parameters = {}
context = <clickhouse_sqlalchemy.drivers.native.base.ClickHouseExecutionContext object at 0x7b54060c91c0>

    def execute(self, operation, parameters=None, context=None):
        self._reset_state()
        self._begin_query()
    
        try:
            execute, execute_kwargs = self._prepare(context)
>           response = execute(
                operation, params=parameters, with_column_types=True,
                **execute_kwargs
            )

clickhouse_sqlalchemy/drivers/native/connector.py:153: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.venv/lib/python3.9/site-packages/clickhouse_driver/client.py:382: in execute
    rv = self.process_ordinary_query(
.venv/lib/python3.9/site-packages/clickhouse_driver/client.py:580: in process_ordinary_query
    return self.receive_result(with_column_types=with_column_types,
.venv/lib/python3.9/site-packages/clickhouse_driver/client.py:212: in receive_result
    return result.get_result()
.venv/lib/python3.9/site-packages/clickhouse_driver/result.py:50: in get_result
    for packet in self.packet_generator:
.venv/lib/python3.9/site-packages/clickhouse_driver/client.py:228: in packet_generator
    packet = self.receive_packet()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <clickhouse_driver.client.Client object at 0x7b54063ca760>

    def receive_packet(self):
        packet = self.connection.receive_packet()
    
        if packet.type == ServerPacketTypes.EXCEPTION:
>           raise packet.exception
E           clickhouse_driver.errors.ServerException: Code: 44.
E           DB::Exception: Cannot create column with type 'JSON' because experimental JSON type is not allowed. Set setting allow_experimental_json_type = 1 in order to allow it. Stack trace:
E           
E           0. DB::Exception::Exception(DB::Exception::MessageMasked&&, int, bool) @ 0x000000000cf7c73b
E           1. DB::Exception::Exception(PreformattedMessage&&, int) @ 0x0000000007ea788c
E           2. DB::Exception::Exception<String>(int, FormatStringHelperImpl<std::type_identity<String>::type>, String&&) @ 0x0000000007ea73cb
E           3. DB::validateDataType(std::shared_ptr<DB::IDataType const> const&, DB::DataTypeValidationSettings const&)::$_0::operator()(DB::IDataType const&) const (.llvm.2336510096372259121) @ 0x00000000117810cd
E           4. DB::InterpreterCreateQuery::getTablePropertiesAndNormalizeCreateQuery(DB::ASTCreateQuery&, DB::LoadingStrictnessLevel) const @ 0x000000001114e26f
E           5. DB::InterpreterCreateQuery::createTable(DB::ASTCreateQuery&) @ 0x00000000111550bf
E           6. DB::InterpreterCreateQuery::execute() @ 0x0000000011167bb0
E           7. DB::executeQueryImpl(char const*, char const*, std::shared_ptr<DB::Context>, DB::QueryFlags, DB::QueryProcessingStage::Enum, DB::ReadBuffer*) @ 0x00000000117351c3
E           8. DB::executeQuery(String const&, std::shared_ptr<DB::Context>, DB::QueryFlags, DB::QueryProcessingStage::Enum) @ 0x0000000011730ffa
E           9. DB::TCPHandler::runImpl() @ 0x0000000012922614
E           10. DB::TCPHandler::run() @ 0x000000001293da78
E           11. Poco::Net::TCPServerConnection::start() @ 0x000000001580b827
E           12. Poco::Net::TCPServerDispatcher::run() @ 0x000000001580bcb9
E           13. Poco::PooledThread::run() @ 0x00000000157d8821
E           14. Poco::ThreadImpl::runnableEntry(void*) @ 0x00000000157d6ddd
E           15. ? @ 0x0000714ceb0b3609
E           16. ? @ 0x0000714ceafd8353

.venv/lib/python3.9/site-packages/clickhouse_driver/client.py:245: ServerException

During handling of the above exception, another exception occurred:

self = <tests.types.test_json.JSONTestCaseNative testMethod=test_select_insert>

    def test_select_insert(self):
        data = {'k1': 1, 'k2': '2', 'k3': True}
    
        self.table.drop(bind=self.session.bind, if_exists=True)
        try:
            # http session is unsupport
            self.session.execute(
                text('SET allow_experimental_object_type = 1;')
            )
>           self.session.execute(text(self.compile(CreateTable(self.table))))

tests/types/test_json.py:48: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py:2229: in execute
    return self._execute_internal(
.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py:2133: in _execute_internal
    result = conn.execute(
.venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py:1414: in execute
    return meth(
.venv/lib/python3.9/site-packages/sqlalchemy/sql/elements.py:489: in _execute_on_connection
    return connection._execute_clauseelement(
.venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py:1638: in _execute_clauseelement
    ret = self._execute_context(
.venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py:1842: in _execute_context
    return self._exec_single_context(
.venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py:1983: in _exec_single_context
    self._handle_dbapi_exception(
.venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py:2328: in _handle_dbapi_exception
    raise exc_info[1].with_traceback(exc_info[2])
.venv/lib/python3.9/site-packages/sqlalchemy/engine/base.py:1964: in _exec_single_context
    self.dialect.do_execute(
clickhouse_sqlalchemy/drivers/base.py:499: in do_execute
    cursor.execute(statement, parameters, context=context)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <clickhouse_sqlalchemy.drivers.native.connector.Cursor object at 0x7b54060c9520>
operation = 'CREATE TABLE test (x JSON) ENGINE = Memory'
parameters = {}
context = <clickhouse_sqlalchemy.drivers.native.base.ClickHouseExecutionContext object at 0x7b54060c91c0>

    def execute(self, operation, parameters=None, context=None):
        self._reset_state()
        self._begin_query()
    
        try:
            execute, execute_kwargs = self._prepare(context)
            response = execute(
                operation, params=parameters, with_column_types=True,
                **execute_kwargs
            )
    
        except DriverError as orig:
>           raise DatabaseException(orig)
E           clickhouse_sqlalchemy.exceptions.DatabaseException: Orig exception: Code: 44.
E           DB::Exception: Cannot create column with type 'JSON' because experimental JSON type is not allowed. Set setting allow_experimental_json_type = 1 in order to allow it. Stack trace:
E           
E           0. DB::Exception::Exception(DB::Exception::MessageMasked&&, int, bool) @ 0x000000000cf7c73b
E           1. DB::Exception::Exception(PreformattedMessage&&, int) @ 0x0000000007ea788c
E           2. DB::Exception::Exception<String>(int, FormatStringHelperImpl<std::type_identity<String>::type>, String&&) @ 0x0000000007ea73cb
E           3. DB::validateDataType(std::shared_ptr<DB::IDataType const> const&, DB::DataTypeValidationSettings const&)::$_0::operator()(DB::IDataType const&) const (.llvm.2336510096372259121) @ 0x00000000117810cd
E           4. DB::InterpreterCreateQuery::getTablePropertiesAndNormalizeCreateQuery(DB::ASTCreateQuery&, DB::LoadingStrictnessLevel) const @ 0x000000001114e26f
E           5. DB::InterpreterCreateQuery::createTable(DB::ASTCreateQuery&) @ 0x00000000111550bf
E           6. DB::InterpreterCreateQuery::execute() @ 0x0000000011167bb0
E           7. DB::executeQueryImpl(char const*, char const*, std::shared_ptr<DB::Context>, DB::QueryFlags, DB::QueryProcessingStage::Enum, DB::ReadBuffer*) @ 0x00000000117351c3
E           8. DB::executeQuery(String const&, std::shared_ptr<DB::Context>, DB::QueryFlags, DB::QueryProcessingStage::Enum) @ 0x0000000011730ffa
E           9. DB::TCPHandler::runImpl() @ 0x0000000012922614
E           10. DB::TCPHandler::run() @ 0x000000001293da78
E           11. Poco::Net::TCPServerConnection::start() @ 0x000000001580b827
E           12. Poco::Net::TCPServerDispatcher::run() @ 0x000000001580bcb9
E           13. Poco::PooledThread::run() @ 0x00000000157d8821
E           14. Poco::ThreadImpl::runnableEntry(void*) @ 0x00000000157d6ddd
E           15. ? @ 0x0000714ceb0b3609
E           16. ? @ 0x0000714ceafd8353

clickhouse_sqlalchemy/drivers/native/connector.py:159: DatabaseException
======================= warnings summary =======================
tests/types/test_enum16.py:11
  .../clickhouse-sqlalchemy/tests/types/test_enum16.py:11: PytestCollectionWarning: cannot collect test class 'TestEnum' because it has a __new__ constructor (from: tests/types/test_enum16.py)
    class TestEnum(enum.IntEnum):

tests/types/test_enum8.py:11
  .../clickhouse-sqlalchemy/tests/types/test_enum8.py:11: PytestCollectionWarning: cannot collect test class 'TestEnum' because it has a __new__ constructor (from: tests/types/test_enum8.py)
    class TestEnum(enum.IntEnum):

tests/types/test_date32.py::Date32TestCaseHTTP::test_select_insert
  .../clickhouse-sqlalchemy/tests/types/test_date32.py:42: SAWarning: Class Select will not make use of SQL compilation caching as it does not set the 'inherit_cache' attribute to ``True``.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this object can make use of the cache key generated by the superclass.  Alternatively, this attribute may be set to False which will disable this warning. (Background on this error at: https://sqlalche.me/e/20/cprf)
    result = self.session.execute(self.table.select()).scalar()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

@9en9i , the #357 must be dealt first, you help is highly appreciated.

I'll convert this PR to draft again.

@stankudrow stankudrow marked this pull request as draft February 5, 2025 17:25
@stankudrow stankudrow mentioned this pull request Feb 5, 2025
5 tasks
@stankudrow stankudrow marked this pull request as ready for review February 6, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop Python 3.8 support
2 participants