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

MySqlHook constructor fails with exception after upgrading apache-airflow-providers-common-sql from 1.16.0 to 1.17.0 #42452

Closed
1 of 2 tasks
agreenburg opened this issue Sep 24, 2024 · 12 comments · Fixed by #42490

Comments

@agreenburg
Copy link

Apache Airflow Provider(s)

common-sql, mysql

Versions of Apache Airflow Providers

apache-airflow-providers-common-sql 1.17.0
apache-airflow-providers-mysql 5.6.1

Apache Airflow version

2.9.2+astro.1

Operating System

debian astronomer/astro-runtime:11.5.0

Deployment

Astronomer

Deployment details

This is an Astronomer-hosted deployment. Docker image is quay.io/astronomer/astro-runtime:11.5.0

What happened

DAGs that interact with MySqlHook started failing after apache-airflow-providers-common-sql was automatically upgraded from 1.16.0 to 1.17.0.

Here is an example error from the log:

[2024-09-24, 18:24:30 UTC] {taskinstance.py:2905} ERROR - Task failed with exception
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/airflow/models/taskinstance.py", line 465, in _execute_task
    result = _execute_callable(context=context, **execute_callable_kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/airflow/models/taskinstance.py", line 432, in _execute_callable
    return execute_callable(context=context, **execute_callable_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/airflow/models/baseoperator.py", line 401, in wrapper
    return func(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/airflow/operators/python.py", line 235, in execute
    return_value = self.execute_callable()
                   ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/airflow/operators/python.py", line 252, in execute_callable
    return self.python_callable(*self.op_args, **self.op_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/airflow/dags/sftp_handler.py", line 25, in getDBConnection
    return MySqlHook(connection_name).get_conn()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/airflow/providers/mysql/hooks/mysql.py", line 75, in __init__
    self.connection = kwargs.pop("connection", None)
    ^^^^^^^^^^^^^^^
AttributeError: property 'connection' of 'MySqlHook' object has no setter

requirements.txt had apache-airflow-providers-mysql with no version constraint.

apache-airflow-providers-mysql

Forcing downgrade to the previous version fixes the problem and allows the DAGs to run again:

apache-airflow-providers-mysql
apache-airflow-providers-common-sql <1.17

What you think should happen instead

Because we are including apache-airflow-providers-mysql without a version constraint, I would hope and expect that it would only bring in a version of apache-airflow-providers-common-sql that is compatible with the version of apache-airflow-providers-mysql that is being used. That does not appear to be happening here.

How to reproduce

  1. Install these exact dependency versions:

apache-airflow-providers-common-sql 1.17.0
apache-airflow-providers-mysql 5.6.1

  1. Inside a python operator, attempt to get an instance of a MysqlHook with a named connection:
conn = MySqlHook("my_named_mysql_connection").get_conn()

This will throw an AttributeError:

AttributeError: property 'connection' of 'MySqlHook' object has no setter

Anything else

This is consistently reproducible with the listed provider versions.

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@agreenburg agreenburg added area:providers kind:bug This is a clearly a bug needs-triage label for new issues that we didn't triage yet labels Sep 24, 2024
Copy link

boring-cyborg bot commented Sep 24, 2024

Thanks for opening your first issue here! Be sure to follow the issue template! If you are willing to raise PR to address this issue please do so, no need to wait for approval.

@agreenburg agreenburg changed the title MySqlHook constructor fails with exception after upgrade from apache-airflow-providers-common-sql from 1.16.0 to 1.17.0 MySqlHook constructor fails with exception after upgrading apache-airflow-providers-common-sql from 1.16.0 to 1.17.0 Sep 24, 2024
@steffidlf
Copy link
Contributor

+1, we have the same issue with the same versions

@dweaver33
Copy link

+1, we're having the same issue as well

@veer0318
Copy link

veer0318 commented Sep 25, 2024

+1 same issue;

quick solution that worked for us (don't know why, to be honest), doing a pip install with this:

apache-airflow-providers-mysql>=5.7.1

@tboschtxr
Copy link

+1 same issue but for postgres

    self.connection: Connection | None = kwargs.pop("connection", None)
AttributeError: can't set attribute 'connection'```

@rawwar
Copy link
Collaborator

rawwar commented Sep 25, 2024

@tboschtxr , can you use apache-airflow-providers-postgres==5.13.0 in you requirements.txt?

@tboschtxr
Copy link

@rawwar

I will give this a try now.

@tboschtxr
Copy link

@rawwar this did work yes, thank you!

@agreenburg
Copy link
Author

This looks like it is resolved with the latest version of apache-airflow-providers-mysql.

However I would like to leave this open for visibility because having dependencies defined without version constraints between providers that might be updated independently could cause things like this to break again in the future due to pip install behavior to not update existing modules that already satisfy the requirement.

Because apache-airflow-providers-mysql has no upper version constraint on its apache-airflow-providers-common-sql dependency, any breaking change to a public API in a future version of apache-airflow-providers-common-sql can break existing airflow installs.

@RNHTTR RNHTTR added provider:postgres and removed needs-triage label for new issues that we didn't triage yet labels Sep 25, 2024
@Lee-W
Copy link
Member

Lee-W commented Sep 26, 2024

Just traced the code a bit. This was due to a change in apache-airflow-providers-mysql==1.17.0. connection is now a descriptor without a setter (and I think this is by design).

The reason why apache-airflow-providers-mysql==5.7.1 works is due to the fix in the same PR. Also postgres change here along with other providers in that PR

@Lee-W
Copy link
Member

Lee-W commented Sep 26, 2024

I'll send a PR for adding a dummy setter for backcompat.

@dabla
Copy link
Contributor

dabla commented Sep 26, 2024

This is probably due to my PR 40751, a dummy setter would indeed solve the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants