Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Django 1.8 support #107

Merged
merged 15 commits into from
Jun 7, 2015
Merged

Django 1.8 support #107

merged 15 commits into from
Jun 7, 2015

Conversation

nemesifier
Copy link
Member

  • fix crappy GEODJANGO_INSTALLED redundant flag
  • run tests with django 1.8 and see what breaks
  • find solution for transaction.is_managed
  • cannot import django.contrib.gis.db.models.sql.query
  • ensure HStoreGeoQuerySet works
  • InterfaceError: connection already closed
  • test_reload_schema fails

Work in progress..

@nemesifier nemesifier mentioned this pull request Apr 18, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+0.62%) to 92.63% when pulling 5d6ab87 on django-1.8 into a78545f on master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.62%) to 92.63% when pulling 5d6ab87 on django-1.8 into a78545f on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.62%) to 92.63% when pulling 5d6ab87 on django-1.8 into a78545f on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.62%) to 92.63% when pulling 5d6ab87 on django-1.8 into a78545f on master.

@landscape-bot
Copy link

Code Health
Repository health increased by 0.48% when pulling 5d6ab87 on django-1.8 into a78545f on master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 5d6ab87 on djangonauts:django-1.8 into 1beec30 on djangonauts:master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 1% when pulling 5d6ab87 on django-1.8 into 0ac2b02 on master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 6ebff43 on djangonauts:django-1.8 into 8bf5a1b on djangonauts:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.76%) to 91.98% when pulling 6ebff43 on django-1.8 into 8bf5a1b on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.76%) to 91.98% when pulling 6ebff43 on django-1.8 into 8bf5a1b on master.

@xeor
Copy link

xeor commented Apr 25, 2015

Is this coming out in django-hstore v1.4? Any ETA?

@nemesifier
Copy link
Member Author

i'm working on it in my free time, I can't provide an ETA.

@nemesifier nemesifier added this to the 1.4 milestone Apr 25, 2015
@xeor
Copy link

xeor commented Apr 25, 2015

As a side-note.. Can you please add a paypal donate button or something on your site? I totally understand that you are doing this in your free time. Just wanted to buy you a beer :) Appreciate your work!

@nemesifier
Copy link
Member Author

very kind of you @xeor, but haven't felt the need of donations yet. I might consider this in the future.
Many people have contributed to this library, I think they'll come back to contribute as soon as they'll need this library to work with their django 1.8 project, unless I finish earlier.

@aidanlister
Copy link
Contributor

So now that there's a HStoreField, will django-hstore wrap that to provide the extra functionality like schema?
https://docs.djangoproject.com/en/1.8/ref/contrib/postgres/fields/#hstorefield

@nemesifier
Copy link
Member Author

@aidanlister: probably in a later release, I've analyzed the amount of work to do this and it will take quite few weeks of work. A gradual implementation sounds more realistic.

@bijanvakili
Copy link
Contributor

@nemesisdesign +1

The work you've done with schema and virtual fields is awesome. Since these don't come with django.contrib.postgres, we'd rather stick with django-hstore. If you need help with incrementally supporting Django 1.8, please feel free to reach out.

@aidanlister
Copy link
Contributor

Just as a note for 1.8ers, you can now enable hstore in a migration:

from django.contrib.postgres.operations import HStoreExtension

class Migration(migrations.Migration):
...

operations = [
    HStoreExtension(),
    ...
]

@nemesifier
Copy link
Member Author

@bijanvakili yes help is appreciated as right now i cannot work on this, if anyone has a bit of time can just proceed on this branch, i'll give him/her write access to push to the 1.8 branch

@aidanlister
Copy link
Contributor

I'm testing the new 1.8 functionality in our staging environments now, soon to move into production which is a little scary.

The only incompatibility we've noticed at the initialisation level is having HStoreExtension in your migrations doesn't help you, because register_hstore_handler (https://github.com/djangonauts/django-hstore/blob/master/django_hstore/apps.py#L42) runs first and crashes out.

You'll need to set up hstore before the connection is established if you want to keep using django-hstore.

@nemesisdesign I might be able to spare a developer for a few days to work on 1.8 compatibility, there does seem to be a 1.8 blocker which is that ConcreteField issue I posted.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.03% when pulling 4b60bd5 on django-1.8 into 641c2e0 on master.

@nemesifier
Copy link
Member Author

After fd33e31 and 4b60bd5 there are a few more issues we can work on.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.94%) to 91.8% when pulling 4b60bd5 on django-1.8 into 641c2e0 on master.

@aidanlister
Copy link
Contributor

Traceback:
File "/app/.heroku/python/lib/python3.4/site-packages/django/core/handlers/base.py" in get_response
  132.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/contrib/admin/options.py" in wrapper
  616.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/utils/decorators.py" in _wrapped_view
  110.                     response = view_func(request, *args, **kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  57.         response = view_func(request, *args, **kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/contrib/admin/sites.py" in inner
  233.             return view(request, *args, **kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/utils/decorators.py" in _wrapper
  34.             return bound_func(*args, **kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/utils/decorators.py" in _wrapped_view
  110.                     response = view_func(request, *args, **kwargs)
File "/app/.heroku/python/lib/python3.4/site-packages/django/utils/decorators.py" in bound_func
  30.                 return func.__get__(self, type(self))(*args2, **kwargs2)
File "/app/.heroku/python/lib/python3.4/site-packages/django/contrib/admin/options.py" in changelist_view
  1655.             selection_note=_('0 of %(cnt)s selected') % {'cnt': len(cl.result_list)},
File "/app/.heroku/python/lib/python3.4/site-packages/django/db/models/query.py" in __len__
  144.         self._fetch_all()
File "/app/.heroku/python/lib/python3.4/site-packages/django/db/models/query.py" in _fetch_all
  965.             self._result_cache = list(self.iterator())
File "/app/.heroku/python/lib/python3.4/site-packages/django/db/models/query.py" in iterator
  255.             obj = model_cls.from_db(db, init_list, row[model_fields_start:model_fields_end])
File "/app/.heroku/python/lib/python3.4/site-packages/django/db/models/base.py" in from_db
  489.             new = cls(*values)
File "/app/.heroku/python/lib/python3.4/site-packages/django/db/models/base.py" in __init__
  482.         signals.post_init.send(sender=self.__class__, instance=self)
File "/app/.heroku/python/lib/python3.4/site-packages/django/dispatch/dispatcher.py" in send
  201.             response = receiver(signal=self, sender=sender, **named)
File "/app/abas/apps/siteconfig/signals/handlers.py" in update_schema
  27.         field.reload_schema(schema)
File "/app/.heroku/python/lib/python3.4/site-packages/django_hstore/fields.py" in reload_schema
  185.         self._remove_hstore_virtual_fields()
File "/app/.heroku/python/lib/python3.4/site-packages/django_hstore/fields.py" in _remove_hstore_virtual_fields
  209.                 getattr(cls._meta, meta_fields).remove(field)
File "/app/.heroku/python/lib/python3.4/site-packages/django/utils/datastructures.py" in complain
  500.             raise AttributeError(self.warning)

Exception Type: AttributeError at /admin/properties/property/
Exception Value: The return type of 'fields' should never be mutated. If you want to manipulate this list for your own use, make a copy first.

Because we're dealing with the new _meta API, removing fields is a no-no. reload_schema removes all the hstore fields and re-adds them which isn't allowed.

@aidanlister
Copy link
Contributor

Alright I think this commit fixes the above: https://github.com/ABASystems/django-hstore/commit/4f295149016e80423e100bba31fc748b49cff9ac

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.03% when pulling 3844307 on django-1.8 into b31f891 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.94%) to 91.8% when pulling 3844307 on django-1.8 into b31f891 on master.

@coveralls
Copy link

coveralls commented Jun 7, 2015

Coverage Status

Coverage decreased (-0.9%) to 91.802% when pulling 3844307 on django-1.8 into b31f891 on master.

@nemesifier
Copy link
Member Author

@aidanlister thx for https://github.com/ABASystems/django-hstore/commit/4f295149016e80423e100bba31fc748b49cff9ac

regarding the connection, django's HStoreField is very similar to django-hstore, see this comment: #76 (comment)

I just pushed some more commits, let's wait and see.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.10% when pulling 8af4c0b on django-1.8 into b31f891 on master.

1 similar comment
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.10% when pulling 8af4c0b on django-1.8 into b31f891 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.45%) to 92.29% when pulling 8af4c0b on django-1.8 into b31f891 on master.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.02% when pulling f9462a0 on django-1.8 into b31f891 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.54%) to 93.28% when pulling f9462a0 on django-1.8 into b31f891 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.54%) to 93.28% when pulling f9462a0 on django-1.8 into b31f891 on master.

nemesifier added a commit that referenced this pull request Jun 7, 2015
@nemesifier nemesifier merged commit 355388c into master Jun 7, 2015
@nemesifier
Copy link
Member Author

Now please help us testing it in the real world!

@aidanlister
Copy link
Contributor

Is there any reason you're checking fields and other_fields rather than virtual_fields?

@nemesifier
Copy link
Member Author

@aidanlister could you be more specific?

@aidanlister
Copy link
Contributor

You're iterating fields, local_fields and virtual_fields ... I think you can just iterate virtual_fields: https://github.com/djangonauts/django-hstore/blob/master/django_hstore/fields.py#L215

See https://github.com/ABASystems/django-hstore/blob/django-1.8/django_hstore/fields.py#L204

@nemesifier
Copy link
Member Author

It's a bit more complex than that. simple virtual fields did not work in the admin by default because inconsistencies between django models and django modeladmin, so I had to pretend virtual fields are also real fields, that's why i was iterating also on other lists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants