coleifer / peewee

a small, expressive orm -- supports postgresql, mysql, sqlite and cockroachdb
http://docs.peewee-orm.com/
MIT License
11.06k stars 1.37k forks source link

Unexpected argument 'named_cursor' after upgrade to 3.17.6 #2914

Closed cieszdan closed 2 months ago

cieszdan commented 2 months ago

Hi, after upgrade I have error during executing requests to DB. Below Traceback

Traceback (most recent call last):
  File "/code/src/services/auth_service.py", line 101, in log_in_user
    user = get_user_with_credentials(email=payload.email, password=hashed_password)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/code/src/database/dals/user_dal.py", line 135, in get_user_with_credentials
    return _get_user_with_roles(query)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/code/src/database/dals/user_dal.py", line 122, in _get_user_with_roles
    users = _get_users_with_roles(query)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/code/src/database/dals/user_dal.py", line 129, in _get_users_with_roles
    return prefetch(query, UserRole)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 8202, in prefetch
    for instance in pq.query:
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 7275, in __iter__
    self.execute()
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 2036, in inner
    return method(self, database, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 2107, in execute
    return self._execute(database)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 2280, in _execute
    cursor = database.execute(self)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/playhouse/postgres_ext.py", line 503, in execute
    cursor = self.execute_sql(sql, params, named_cursor=named_cursor)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: ReconnectMixin.execute_sql() got an unexpected keyword argument 'named_cursor'
class ReconnectMixin(object):
    """
    Mixin class that attempts to automatically reconnect to the database under
    certain error conditions.

    For example, MySQL servers will typically close connections that are idle
    for 28800 seconds ("wait_timeout" setting). If your application makes use
    of long-lived connections, you may find your connections are closed after
    a period of no activity. This mixin will attempt to reconnect automatically
    when these errors occur.

    This mixin class probably should not be used with Postgres (unless you
    REALLY know what you are doing) and definitely has no business being used
    with Sqlite. If you wish to use with Postgres, you will need to adapt the
    `reconnect_errors` attribute to something appropriate for Postgres.
    """
    reconnect_errors = (
        # Error class, error message fragment (or empty string for all).
        (OperationalError, '2006'),  # MySQL server has gone away.
        (OperationalError, '2013'),  # Lost connection to MySQL server.
        (OperationalError, '2014'),  # Commands out of sync.
        (OperationalError, '4031'),  # Client interaction timeout.

        # mysql-connector raises a slightly different error when an idle
        # connection is terminated by the server. This is equivalent to 2013.
        (OperationalError, 'MySQL Connection not available.'),

        # Postgres error examples:
        #(OperationalError, 'terminat'),
        #(InterfaceError, 'connection already closed'),
    )

    def __init__(self, *args, **kwargs):
        super(ReconnectMixin, self).__init__(*args, **kwargs)

        # Normalize the reconnect errors to a more efficient data-structure.
        self._reconnect_errors = {}
        for exc_class, err_fragment in self.reconnect_errors:
            self._reconnect_errors.setdefault(exc_class, [])
            self._reconnect_errors[exc_class].append(err_fragment.lower())

    def execute_sql(self, sql, params=None, commit=None):
        if commit is not None:
            __deprecated__('"commit" has been deprecated and is a no-op.')
        return self._reconnect(super(ReconnectMixin, self).execute_sql, sql, params)

Missing params in shortcuts.py in playhouse

coleifer commented 2 months ago

Why are you using the MySQL reconnect mixin with Postgres?

cieszdan commented 2 months ago

This will work for postgres as well if you changed reconnect_errors object

coleifer commented 2 months ago

That is correct, but since you're already subclassing and extending the ReconnectMixin you will also need to override the signature now for the execute_sql().

The docstring indicates that this should really not be used with Postgres, and that remains the case.

elderlabs commented 3 weeks ago

My follow up to this would then be what is the appropriate thing to use in order to reconnect when the connection to the PSQL database drops out? We've used the MySQL reconnect mixin for years without an issue and it's been extremely useful, otherwise our process needs to reboot in order to reestablish its connection, which may take a considerable amount of time. With the release of 3.17.6, it would seem we're left without a working solution, unless I'm missing something entirely. For the time being, we've rolled back to 3.17.5 until we can find a solution to this issue.

Thank you and regards.

coleifer commented 3 weeks ago

No need to roll back necessarily, you'll just want to ensure that your ReconnectMixin supports that special PostgresqlExtDatabase parameter for named cursor support and override the appropriate error list.

You can try using something like pgbouncer to manage the health of your postgres connections, but even then you may still see an occasional connection issue if you (e.g.) restart pgbouncer.