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

Pybrake query notifier throws error when query is a psycopg2 Composed object #156

Closed
mlennon-spr opened this issue May 18, 2021 · 1 comment · Fixed by #159
Closed

Pybrake query notifier throws error when query is a psycopg2 Composed object #156

mlennon-spr opened this issue May 18, 2021 · 1 comment · Fixed by #159
Assignees

Comments

@mlennon-spr
Copy link

    import pybrake

    notifier = pybrake.Notifier(project_id='test',
                                project_key='test',
                                )
    query = sql.SQL("""
               SELECT
                   1
               WHERE
                   1 in {ids}
               """).format(ids=sql.Literal((1,2,3)))
    notifier.queries.notify(
        query=query,
        method='request.method',
        route='test-route',
        start_time=time.time(),
        end_time=time.time(),
    )

Raises an error: unhashable type: 'Composed':

File "/usr/local/lib/python3.7/site-packages/pybrake/queries.py" in notify
  61.             if key in self._stats:
Exception Value: unhashable type: 'Composed'

This seems to arise because query_stat_key def query_stat_key(*, query="", method="", route="", time=None): is used to create a dictionary key, which would only be valid if the query object is hashable.

@scottsbaldwin
Copy link

@shifi can you take a look at this issue and the related PR?

shifi added a commit that referenced this issue May 24, 2021
Fixes #156. Replaces #157.

This updates pybrake's wrapper `CursorWrapper` to convert SQL Composed
objects into strings before sending query stats to Airbrake. Converting
them to strings will result in better query readability on the
Performance Dashboard. There was also a bug where the notifier would
raise an error because it was not able to process the Composed object.
With this change, that should be fixed.

Big thanks to @mlennon-spr for reporting the bug and presenting the solution!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants