mopidy / mopidy-local-sqlite

DEPRECATED (Mopidy SQLite local library extension)
https://mopidy.com
Apache License 2.0
30 stars 10 forks source link

SQL Error Aborts Local Scan #20

Closed tkem closed 10 years ago

tkem commented 10 years ago

Contrary to expectation, exceptions raised from local.Library.add() are not caught by mopidy local scan, but abort the import process. Therefore, no exceptions should be raised from SQLiteLibrary.add() at all.

jodal commented 10 years ago

I haven't thought the case through, but if you feel Mopidy should catch these exceptions, please open an issue with Mopidy :-)

tkem commented 10 years ago

First of all, thanks for watching ;-) I'm not sure yet -- I generally have a strong feeling against try-catch blocks that wrap an entire function, but I guess that's just "taste".

tkem commented 10 years ago

There actually may be situations where the local library may want to tell the calling process to just give up and stop it. Referential integrity violations come to mind, where all you can probably do is clear the local lib and start again, hoping for the best. So I guess the current implementation is okay, at least from my POV.

jodal commented 10 years ago

Sounds good :-)

tkem commented 10 years ago

As always, documentation could be improved, I guess ;-)