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

Error Message Formatting : Missing Bind Parameter #4500

Closed
natec425 opened this issue Feb 14, 2019 · 2 comments
Closed

Error Message Formatting : Missing Bind Parameter #4500

natec425 opened this issue Feb 14, 2019 · 2 comments
Labels
engine engines, connections, transactions, isolation levels, execution options feature
Milestone

Comments

@natec425
Copy link
Contributor

Hey hey,

I'm just looking for comments/interest on changing the error formatting for StatementError. I'm interested in making small changes at the two lines below:

  • Change %r to %s so that multi-line SQL statements appear formatted in a way that would be more quickly recognized by its author.

details.append("[SQL: %r]" % self.statement)

  • Join on "\n" instead of " " in hopes of improved readability

return " ".join(["(%s)" % det for det in self.detail] + details)

For example, this makes the difference between the two error messages:

sqlalchemy.exc.StatementError: (sqlalchemy.exc.InvalidRequestError) A value is required for bind parameter 'id' [SQL: 'select * from reviews where id = %(id)s'] (Background on this error at: http://sqlalche.me/e/cd3x)

(sqlalchemy.exc.InvalidRequestError) A value is required for bind parameter 'id' [SQL: 'select * from reviews where id = %(id)s'] (Background on this error at: http://sqlalche.me/e/cd3x)

becomes

sqlalchemy.exc.StatementError: (sqlalchemy.exc.InvalidRequestError) A value is required for bind parameter 'id'
[SQL: (select * from reviews where id = %(id)s)]
(Background on this error at: http://sqlalche.me/e/cd3x)

(sqlalchemy.exc.InvalidRequestError) A value is required for bind parameter 'id'
[SQL: (select * from reviews where id = %(id)s)]
(Background on this error at: http://sqlalche.me/e/cd3x)

If this sounds alright. I'd be happy to make a PR 😄

Thanks for your time.

@zzzeek
Copy link
Member

zzzeek commented Feb 14, 2019

looks nicer but I worry the use case of grepping logs might be impacted. though the error message itself is still staying on one line so maybe it wont be so bad. we're right at the edge of me wanting to release 1.3.0 final so, that's where it would have to be. I'd be happy to evaluate a PR to see what it ends up looking like.

@zzzeek zzzeek added the engine engines, connections, transactions, isolation levels, execution options label Feb 14, 2019
@zzzeek zzzeek added this to the 1.3 milestone Feb 14, 2019
@sqla-tester
Copy link
Collaborator

Nate Clark has proposed a fix for this issue in the master branch:

Issue #4500 : Change StatementError formatting (newlines and %s) https://gerrit.sqlalchemy.org/1136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine engines, connections, transactions, isolation levels, execution options feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants