kennethreitz / records

SQL for Humans™
https://pypi.python.org/pypi/records/
ISC License
7.14k stars 570 forks source link

records python3 is not compatible with new sqlalchemy release 1.4.0 #208

Closed jdonboch closed 4 months ago

jdonboch commented 3 years ago

I am getting the following error when trying to parse the results of a query. Digging into this more, this seems to be an issue with compatibility with the new sqlalchemy that was released this week (March 14, 2021).

There doesn't appear to be any version restrictions defined in the setup.py which allows things like pip to grab the newer 1.4.0 release which is not usable.

The easiest fix would be to update the setup.py to ensure sqlalchemy < 1.4 is used.

Example code:

db_reader = records.Database(DB_URL)
rows = db_reader.query(SQL, fetchall=True, **kwargs)
row_dict = {key: check_type(value) for (key, value) in dict(rows[0]).items()}

Example error:

AttributeError: 'RMKeyView' object has no attribute 'index'

This is thrown from the following line. https://github.com/kenreitz42/records/blob/c23f5770127826665402e17ab4e115127b5489c5/records.py#L54

Seems like the sqlalchemy 1.4.0 changes some interfaces. When I reverted to sqlalchemy 1.3.23, it appears to work again.

jdonboch commented 3 years ago

Perhaps this is a regression on the part of sqlalchemy, they released a patched 1.4.1 today that fixed many regressions but this still seems to be an issue even when using 1.4.1

amunchet commented 3 years ago

When trying to use records for non-select statements (i.e. insert or delete), records appears to be trying to read in the result, which the newest version of sqlalchemy doesn't allow.

Regression to 1.3* sqlalchemy fixes this issue.

If this project is still active, I'll issue a PR for the fix (should be pretty straightforward) - otherwise this comment is to help those facing the same issue.

/usr/local/lib/python3.8/dist-packages/records.py:300: in query
    return conn.query(query, fetchall, **params)
/usr/local/lib/python3.8/dist-packages/records.py:365: in query
    row_gen = (Record(cursor.keys(), row) for row in cursor)
/usr/local/lib/python3.8/dist-packages/sqlalchemy/engine/result.py:938: in __iter__
    return self._iter_impl()
/usr/local/lib/python3.8/dist-packages/sqlalchemy/engine/result.py:638: in _iter_impl
    return self._iterator_getter(self)
/usr/local/lib/python3.8/dist-packages/sqlalchemy/util/langhelpers.py:1162: in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
/usr/local/lib/python3.8/dist-packages/sqlalchemy/engine/result.py:361: in _iterator_getter
    make_row = self._row_getter
/usr/local/lib/python3.8/dist-packages/sqlalchemy/util/langhelpers.py:1162: in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
/usr/local/lib/python3.8/dist-packages/sqlalchemy/engine/result.py:320: in _row_getter
    keymap = metadata._keymap
/usr/local/lib/python3.8/dist-packages/sqlalchemy/engine/cursor.py:1197: in _keymap
    self._we_dont_return_rows()
/usr/local/lib/python3.8/dist-packages/sqlalchemy/engine/cursor.py:1178: in _we_dont_return_rows
    util.raise_(
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    def raise_(
        exception, with_traceback=None, replace_context=None, from_=False
    ):
        r"""implement "raise" with cause support.

        :param exception: exception to raise
        :param with_traceback: will call exception.with_traceback()
        :param replace_context: an as-yet-unsupported feature.  This is
         an exception object which we are "replacing", e.g., it's our
         "cause" but we don't want it printed.    Basically just what
         ``__suppress_context__`` does but we don't want to suppress
         the enclosing context, if any.  So for now we make it the
         cause.
        :param from\_: the cause.  this actually sets the cause and doesn't
         hope to hide it someday.

        """
        if with_traceback is not None:
            exception = exception.with_traceback(with_traceback)

        if from_ is not False:
            exception.__cause__ = from_
        elif replace_context is not None:
            # no good solution here, we would like to have the exception
            # have only the context of replace_context.__context__ so that the
            # intermediary exception does not change, but we can't figure
            # that out.
            exception.__cause__ = replace_context

        try:
>           raise exception
E           sqlalchemy.exc.ResourceClosedError: This result object does not return rows. It has been closed automatically.

/usr/local/lib/python3.8/dist-packages/sqlalchemy/util/compat.py:198: ResourceClosedError
kolapapa commented 3 years ago

I have the same problem. Im using flask-sqlalchemy==2.4.3 in requirements.txt. I didn't pinned the version of sqlalchemy. And Flask-sqlalchemy pinned the sqlalchemy with >=1.2 in prod(at 20210208 published):

pip freeze | grep 'SQLAlchemy'
SQLAlchemy==1.3.23

in prod(at 20210324 published):

pip freeze | grep 'SQLAlchemy'
SQLAlchemy==1.4.2

Temporary solutions is pinned sqlalchemy

SQLAlchemy==1.3.23
bkimup commented 3 years ago

What's the likelihood this gets resolved? Regression to 1.3*, specifically 1.3.24, fixed the issue for me, but would love to know if I should start looking at other options.

jdonboch commented 3 years ago

I think the records library is not maintain anymore, so I think this is very unlikely to be fixed unless someone forks it and begins to maintain. It is unfortunate as the company that I am working has a lot of stuff that relies on it but it's mostly legacy and we avoid using this library for new development.

atleta commented 3 years ago

I'm baffled to see how many libraries don't specify the maximum version number of its dependencies. While there is no surefire way to do it, they should at least defensively rely on the versioning strategy of the dependency (e.g. if those follow semantic versioning). I have a few rearly updated projects that will keep breaking due to this.

kennethreitz commented 4 months ago

A new release has been pushed out, which should resolve this.