piccolo-orm / piccolo

A fast, user friendly ORM and query builder which supports asyncio.
https://piccolo-orm.com/
MIT License
1.44k stars 91 forks source link

Feature request: abstract db driver exceptions to common exceptions provided by piccolo #487

Open scott2b opened 2 years ago

scott2b commented 2 years ago

For consideration: abstract and more semantically meaningful object-centric exceptions

This may be a matter of opinion, but I feel that one of the key features of an ORM is typically to abstract away the db driver stuff so that an application is not littered with driver-specific handling in code that is otherwise business object-centric.

As a case in point, inserting or creating an object with a conflicting index value might raise:

both of which I may need to handle in a typical application. It is not clear to me whether a sync driver is ever used and raises exceptions, or if all sync calls actually use the async driver, but that uncertainty is itself problematic and should probably be documented.

This all becomes a bigger problem if there is a push to support other drivers. It seems typical for ORMs to generally abstract driver exceptions to common, more semantically appropriate exceptions. In this case, something like an Exists, ObjectExists, or similar error, available directly from the Table class would be ideal.

Workaround

I am experimenting with the following workaround in my own code. This seems to be fine so far: Define a tuple that contains the multiple exceptions and then attach that to the Table class with a meaningful name. E.g:

ObjectExistsErrors = (asyncpg.exceptions.UniqueViolationError, sqlite3.IntegrityError)

class MyModel(Table):

    Exists = ObjectExistsErrors

   ...

Which then can be caught as:

try:
    obj = await MyModel.objects().create(...)
except MyModel.Exists:
    ...
dantownsend commented 2 years ago

I totally agree - this needs looking in to. Will start researching it.