search

maxtepkeev / architect

A set of tools which enhances ORMs written in Python with more features
Other
391 stars 57 forks source link

autocommit being on prevents django model from being used inside a transaction. #4

Closed budlight closed 9 years ago

budlight commented 9 years ago

before this change I would get ProgrammingError: autocommit cannot be used inside a transaction when I used with transaction.atomic():

This might need to be a setting of some sort, also might want to put some comments into the reasoning it is turned on by default.

maxtepkeev commented 9 years ago

Hi @budlight!

Can you please show me more code which reproduces the problem you had while using transaction.atomic(). Autocommit was turned on by default mainly because of SQLAlchemy, but if it creates any problem for Django or any other ORM, I can reimplement this part of the code in another way, so all ORMs would be happy, I just need some more details from you to reproduce the problem here.

Thanks.

benjaminrigaud commented 9 years ago

I just got this error while playing tests with Postgresql on Django

maxtepkeev commented 9 years ago

Thanks for the report. Yes, this error is known and confirmed.

Unfortunately I'm very very busy on my main job at the moment, so I can't give the ETA for the fix.

benjaminrigaud commented 9 years ago

We could help you: do you have some tips / ideas how to fix this? Do you think it would be feasible for someone new on this project?

Thank you.

maxtepkeev commented 9 years ago

I didn't think about the proper fix yet, but I can give you some ideas. The error happens because we explicitly set autocommit to True which is needed for SQLAlchemy, because I didn't find any other way to make it execute custom raw SQL statements using cursor object, but as you see this pisses off Django ORM.

The simplest of all possible fixes would be to just pass the flag like what ORM we're using and create a series of IF statements inside which we could turn autocommit to on or off, depending on the ORM being used, but this is a quick and dirty fix, because it hardcodes the ORM's and I really don't like it.

We should create something more abstract, something more object oriented without hardcoding. But this solution needs to be very well thought, and unfortunately as I said I don't have time to think about it at the moment.

As for the somebody new on the project, why not, but at first he/she needs to make several pull requests and I have to see that this pull requests are really smart and we think in the same direction when working on problem solving tasks.

BTW I believe you can use django-db-parti package which doesn't have this problem and when this error will be fixed in architect you can safely switch to it because it is backwards compatible with django-db-parti.

maxtepkeev commented 9 years ago

Fixed in v0.3.0. Please see docs, because Architect was almost completely rewritten.