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

Improve exception handling: Properly raise IntegrityError exceptions #583

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

amotl
Copy link
Member

@amotl amotl commented Sep 28, 2023

About

When receiving DuplicateKeyException errors from CrateDB, SQLAlchemy should raise corresponding IntegrityError exceptions instead of ProgrammingError, because applications expect it this way.

Details

This patch evaluates the area preliminarily, mainly whether it will work by using CrateDB's own variants of the canonical DBAPI exceptions, and whether SQLAlchemy will honor that properly.

References

@amotl amotl force-pushed the amo/error-handling-integrity-error branch from 8c39426 to c4fb76b Compare September 28, 2023 13:51
@amotl amotl requested review from seut and matriv September 28, 2023 13:51
@amotl amotl marked this pull request as ready for review September 28, 2023 13:51
Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a test case for this?
(Codecov also complaints for line 202 not covered by a test)

@matriv matriv self-requested a review September 28, 2023 14:35
@matriv
Copy link
Contributor

matriv commented Sep 28, 2023

Apologies, accidentally I approved already, instead of only comment.

@amotl amotl force-pushed the amo/error-handling-integrity-error branch from c4fb76b to 6717def Compare September 28, 2023 15:07
@amotl
Copy link
Member Author

amotl commented Sep 28, 2023

Thanks. I've just amended the patch to provide a corresponding test case.

... when receiving `DuplicateKeyException` errors from CrateDB, instead
of the more general `ProgrammingError`.
@amotl amotl force-pushed the amo/error-handling-integrity-error branch from 6717def to ea2555c Compare September 28, 2023 15:10
@amotl amotl merged commit ac8b2f4 into master Sep 28, 2023
29 checks passed
@amotl amotl deleted the amo/error-handling-integrity-error branch September 28, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants