Opened 16 months ago

Closed 15 months ago

Last modified 15 months ago

#22321 closed Bug (fixed)

Missing DB errors wrapping around _set_autocommit

Reported by: alexkoshelev Owned by: aaugustin
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Implementation defined _set_autocommit function can raise DatabaseError that will not be wrapped and converted to unified exception class by caller in BaseDatabaseWrapper.set_autocommit.

Change History (9)

comment:1 Changed 16 months ago by alexkoshelev

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 16 months ago by alexkoshelev

  • Summary changed from Missing DB errors wraping around _set_autocommit to Missing DB errors wrapping around _set_autocommit

comment:3 Changed 16 months ago by aaugustin

  • Has patch set
  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

It turns out that I was working in this area today. See https://github.com/django/django/pull/2468.

comment:4 Changed 16 months ago by alexkoshelev

Looks good but I don't understand why we cannot wrap exceptions in the one place – BaseDatabaseWrapper.set_autocommit?

Last edited 16 months ago by alexkoshelev (previous) (diff)

comment:5 Changed 16 months ago by aaugustin

For consistency. If you look at the rest of the class, database-level methods are prefixed with an underscore and that's where this handling belongs.

If a database adapter implementation autocommit in such a way that _set_autocommit couldn't raise an exception, it wouldn't need the exception wrapping (but that's a rather theoretical argument).

comment:6 Changed 16 months ago by alexkoshelev

Oh, ok. It is not so obvious at first glance.

comment:7 Changed 15 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 3becac84845cee8f12a5fb7f68c87cbaf029c6a0:

Fixed #22321 -- Wrapped exceptions in _set_autocommit.

Refs #21202.

comment:8 Changed 15 months ago by Aymeric Augustin <aymeric.augustin@…>

In 746cded010c7ba3e2bf2df4cde32dcc9b75cb0d3:

[1.6.x] Fixed #22321 -- Wrapped exceptions in _set_autocommit.

Refs #21202.

Backport of 3becac84 from master

comment:9 Changed 15 months ago by Aymeric Augustin <aymeric.augustin@…>

In 5f22bda3827f32289793b90a387d8c77d602a859:

[1.7.x] Fixed #22321 -- Wrapped exceptions in _set_autocommit.

Refs #21202.

Backport of 3becac84 from master

Note: See TracTickets for help on using tickets.
Back to Top