Django 1.7 compatibility

[nemesifier]

Trying travis automated testing for django versions: 1.5, 1.6 and 1.7a2, related to #33

[nemesifier]

@Naddiseo post any improvement to this branch (django-1.7), I gave you write access.

[Naddiseo]

Okay, I should be able to get round to fixing this up on the weekend.

[nemesifier]

i had to change the import statement in the tests because there was an error mentioning conflicting models. This django 1.7 thing looks rather complex.. i hope it won't be a pain in the ass.

Now the tests run, we have 11 failures and 10 errors out of 67.

@niwibe adding you to the discussion just to inform you

[fabiocorneti]

The current WhereNode implementation checks if a child has an 'as_sql' method; if this is the case, sql and params are generated by calling child.as_sql instead of self.make_atom. All the new built-in lookups have this method, so HStoreWhereNode.make_atom is never called.

I think the only way to properly support Django 1.7 is to create custom lookups for PostgreSQL/Hstore ( #19 ), as described here: http://django.readthedocs.org/en/latest/ref/models/custom-lookups.html#writing-alternative-implementations-for-existing-lookups

[nemesifier]

as in #19 then?

[fabiocorneti]

I've implemented the comparison operators in my fork at https://github.com/fabiocorneti/django-hstore/tree/django-1.7 ;with these custom lookups the test suite passes using Django 1.7b1 .

[nemesifier]

good, let's work together on this, i give you write access and please push your commits to this branch, i'll rebase/squash if needed when the feature is ready

[nemesifier]

hey @fabiocorneti, could you push your commits to this branch? I gave you write access, I'd like us to work all together on this, I will squash/edit the commit history if needed when the feature is ready

[nemesifier]

thanks @fabiocorneti @niwibe & @mjtamlyn your feedback is very welcome (when you have some free time of course)

[nemesifier]

thanks @mjtamlyn for your feedback

[nemesifier]

@mjtamlyn fabio has refactored according to your suggestion, any other suggestion? @niwibe I think you might be busy, so I'll wait a bit more, I'd like to merge only after your review

[niwinz]

Thanks for everybody involved for add django 1.7 support to django-hstore, very great work! I have revised it now (previously too) and it is fine for me.

It is ready for merge in my opinion \o/

(You are right! Currently I don't have much time, sorry :( )

[nemesifier]

will merge next week if no more suggestions

[fabiocorneti]

I have found another Django 1.7 specific issue; if you run the django-hstore test suite or the test suite of an app that depends on django-hstore using the django.db.backends.postgresql_psycopg2 backend instead of django.contrib.gis.db.backends.postgis, the registration of the hstore type in django_hstore.utils.register_type fails because when the method is called, the connection cursor points to the postgres database, thus the oid's of the hstore types may be missing or different than the oid's of the test suite database.

To replicate the issue in the default test suite, we'd need to run it using two different settings files, do you know if it's possible reconfigure django at runtime and restart the suite? Otherwise we'd need to adjust the Travis configuration to test the app against both backends.

In the meantime I've fixed the issue locally by deferring hstore registration using this code:

def register_hstore_handler(connection, **kwargs):
    # do not register hstore if DB is not postgres
    # do not register if HAS_HSTORE flag is set to false
    if connection.vendor != 'postgresql' or \
       connection.settings_dict.get('HAS_HSTORE', True) is False:
        return

    # if the ``NAME`` of the database in the connection settings is ``None``
    # defer hstore registration by setting up a new unique handler
    if connection.settings_dict['NAME'] is None:
        connection_handler.attach_handler(register_hstore_handler,
                                          vendor="postgresql", unique=True)
        return

    if sys.version_info[0] < 3:
        register_hstore(connection.connection, globally=True, unicode=True)
    else:
        register_hstore(connection.connection, globally=True)

connection_handler.attach_handler(register_hstore_handler,
                                  vendor="postgresql", unique=True)

If it looks good I will push the change.

[nemesifier]

better run another test suite with psycopg2 backend, sending a commit in a moment

[niwinz]

@fabiocorneti is not bad solution, seems reasoble.

In any case, as I commented on related #35 issue, an other solution is doccumenting that for avoid problems, hstore extension should be installed on template1.

[nemesifier]

@fabiocorneti running tests with psycopg2 backend does not reproduce the issue in my environment. I pushed the changes so they will be tried on travis too.

Waiting for this build: https://travis-ci.org/djangonauts/django-hstore/builds/22964480

Please try pulling the changes and running ./runtests.py --settings=settings_psycopg2

[nemesifier]

ok now I see some issues

[fabiocorneti]

@nemesisdesign the issue happens if the hstore extension is not installed in the postgres database or if the oids of the hstore types in the postgres database are different than the oids of the hstore types in the template1/app database; if I run the test suite using settings_psycopg locally, I get the same error displayed here:

https://travis-ci.org/djangonauts/django-hstore/jobs/22964484#L4851

If I defer the registration, the error goes away.

[nemesifier]

send a commit with the change

[fabiocorneti]

I'll send the commit soon, trying to fix the 1.5 test runner issue as well :)

[nemesifier]

@fabiocorneti pull latest commit before push to avoid merge commit

[nemesifier]

@fabiocorneti just ignore, i'll take care of it

[nemesifier]

ok all travis builds pass now, I think this is ready to merge, let me know what you people think.

[KristianOellegaard]

Looks good to me - will be great to have the regular psycopg2 backend tested in travis as well!

[nemesifier]

merged