laughingman7743 / PyAthena

PyAthena is a Python DB API 2.0 (PEP 249) client for Amazon Athena.
MIT License
463 stars 105 forks source link

Breaking change in the release between 3.0.10 and 3.1.0 #506

Closed cgramberg-indeed closed 9 months ago

cgramberg-indeed commented 9 months ago

Hello, I have found that there is a breaking change here in a recent release where the Cursor class (and I think all the built in cursors) no longer set default values for any of these arguments : s3_staging_dir, schema_name, catalog_name, work_group, poll_interval, encryption_option, kms_key, kill_on_interrupt, result_reuse_enable, result_reuse_minutes.

I'm thinking that at least the base Cursor class needs these default values. Here is an example of code that would function in 3.0.10 but breaks in 3.1.0:

cursor = Cursor(
    connection=Connection(),
    converter=DefaultTypeConverter(),
    formatter=DefaultParameterFormatter(),
    retry_config=RetryConfig(),
)

This will cause a TypeError: __init__() missing 10 required positional arguments: 's3_staging_dir', 'schema_name', 'catalog_name', 'work_group', 'poll_interval', 'encryption_option', 'kms_key', 'kill_on_interrupt', 'result_reuse_enable', and 'result_reuse_minutes'

Normally it is not an issue because the Connection handles initializing the Cursor but we have a subclass of Cursor which we were initializing directly in our unit tests.

laughingman7743 commented 9 months ago

This error is affected by the following changes: https://github.com/laughingman7743/PyAthena/pull/504 Do you have any good ideas to fix this error?

laughingman7743 commented 9 months ago

https://github.com/laughingman7743/PyAthena/pull/507

woosuk-choi-g commented 9 months ago

This error is affected by the following changes: #504 Do you have any good ideas to fix this error?

In my opinion, I believe this issue arose due to this commit.

The default value has been removed from some constructor parameters of the class Cursor.

laughingman7743 commented 9 months ago

Ah, you are right, it is not the most recent type hint change.

laughingman7743 commented 9 months ago

I need to fix it so that each cursor sets a default value. I will fix it soon.

cgramberg-indeed commented 9 months ago

I need to fix it so that each cursor sets a default value. I will fix it soon.

I agree that this is the ideal fix! Thank you for the fast reply!