Better error message when trying to drop constraints/indexes in a transaction block

[theGOTOguy]

This will fail with an opaque error for all but very trivial databases, so we throw a more meaningful error right away instead of letting this fail at the point of dropping constraints/indexes in order to avoid confusion.

#95

[coveralls]

Coverage increased (+0.02%) to 98.834% when pulling 248c88e on theGOTOguy:better_error into b27230f on california-civic-data-coalition:master.

[theGOTOguy]

It turns out that many of your existing tests are predicated on the success of drop_constraints and drop_indexes inside a transaction block because the test databases are simple enough not to break when you do so. Thus, I've patched out the check for your tests.

Note that this may be a breaking change for some users who are using from_csv with very simple databases inside transactions. However, these same users would not be harmed by explicitly adding drop_constraints=False and drop_indexes=False because their performance hit would be insignificant anyway based on the simplicity of their databases.

I suspect, however, that the vast majority of users will never notice this change because they're not doing this in a transaction block and instead just have it in some management command. The benefit is that new users with non-trivial databases will not be confused by the opaque error message psycopg2 throws at you when you try to use drop_constraints or drop_indexes in a transaction block.

[palewire]

Thanks for these thoughtful patches. I am a little preoccupied with dealing with the upcoming US election at my day job but will try to look at them shortly.

…

On Thu, Oct 25, 2018, 12:56 AM Ben Li-Sauerwine ***@***.***> wrote: It turns out that many of your existing tests are predicated on the success of drop_constraints and drop_indexes inside a transaction block because the test databases are simple enough not to break when you do so. Thus, I've patched out the check for your tests. Note that this may be a breaking change for some users who are using from_csv with very simple databases inside transactions. However, these same users would not be harmed by explicitly adding drop_constraints=False and drop_indexes=False because their performance hit would be insignificant anyway based on the simplicity of their databases. I suspect, however, that the vast majority of users will never notice this change because they're not doing this in a transaction block and instead just have it in some management command. The benefit is that new users with non-trivial databases will not be confused by the opaque error message psycopg2 throws at you when you try to use drop_constraints or drop_indexes in a transaction block. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#99 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAnCXVtOmcpXq5ntsqC0F8BBTaCrztiks5uoW6ngaJpZM4X5nWM> .

[theGOTOguy]

Sure, I'm not in any rush. If you're not interested in this PR or the other one I added, no problem, I don't take it personally.