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
Zinnia breaks on Django 1.7b1: get_user_model at init time #342
Comments
This issue is now on top of the TODO. I never develop against beta release. Pull Request are welcome. |
Hi,
settings.AUTH_USER_MODEL seems to be a str, thus it cannot be used as a baseclass. As a workaround I derived Author from django.contrib.auth.models.User, but this will break if settings.AUTH_USER_MODEL was changed at a site. From https://docs.djangoproject.com/en/dev/topics/auth/customizing/ I would assume that the preferred way would be to use a OneToOneField from Author to User:
|
Sorry thought I had removed this. I will try and post some notes on what I
|
…atch Zinnia v0.14.1, unfortunately. See Fantomas42#342 and 353.
Django 1.7 is released. Issue become critical. |
@aisayko Yes I have seen the release this morning. |
Whilst I agree that there is time to fix this issue before the majority of I think the crux of the matter is that it isn't possible to create a proxy The proxy model needs to be registered as part of the registry but can't be On Wed, Sep 3, 2014 at 1:20 PM, Julien Fache notifications@github.com
|
@benvand I'm perfectly okay with you on all the points. |
@benvand, @hriechmann I think this is a good way to go about it. In addition to adding 1.7 compatibility, doing away with a proxy model will address #369 and will likely make #346 easier to deal with. Also it just makes more sense to me from a design perspective. My settings.AUTH_USER_MODEL is my user model. My site may have a site profile model attached to it and Zinnia has a blog profile (Author) attached to it. From a user perspective I would have a profile for the site and another profile for the blog. With this foundation in place the Zinnia profile (Author) could be done in such a way that a Zinnia implementer could extend/customize aspects of the Author model. E.g. Make its display fields return information from their site's profile. |
So my thinking was you could create the same functionality with a post save signal on the settings.AUTH_USER_MODEL that automatically creates a: class Author(models.Model)
user = models.OneToOneField(settings.AUTH_USER_MODEL)
etc...
etc... but you can't register signals with AUTH_USER_MODEL because you register signals at run time too. Just spitballing here but I suppose you could replace every instance of: Author.objects.get(pk=request.user.id) with an Author.objects.get_or_create(id=request.user.id, user=get_user_model().objects.get(id=request.user.id))[0] Something like that? |
Couldn't this be as simple as I don't think it's a good idea to assume that the author id is in sync with the user id as all kinds of scenarios can prevent that from being the case. It seems safe to me to be able to just assume user=request.user is unique because that relationship should be a one to one. Edit Note: I realize now that you may be assuming they are the same because it might be what the code does now. I think I need to research this a bit more. Edit Note 2: It's ok for the code to assume it now as it's a proxy model and an Author does not maintain it's own record. Whereas in the case of a foreign key it would maintain it's own record (because Author would become a non-proxy, standard model). So I guess my initial concern about assuming the author and user id is the same is accurate (if the Author would no longer be a proxy). |
I think this may work. You're right about the id of author objects not mattering. Proxy model. For posterity what we need to do is replace the proxy model models.author.Author with a standard model that whilst containing all the functionality of the proxy model also has a one-to-one relationship with settings.AUTH_USER_MODEL.
In the codebase we need to reflect this refactor accessing the author model specifically using
and not using (for example)
unless a DoesNotExist try except is implemented @ivanvenosdel @Fantomas42 does that sound about right to everyone? |
Sounds good to me. I am going to take a shot at it today. |
…test file (zinnia.tests.test_feeds) passing.
Once I got started I found that it will be a lot more work then I have time for but I did figure out a few things that should be useful. Here are my initial steps, I had replaced the proxy model with a concrete one and managed to fix a few test files but stopped when I realized the amount of effort involved will be large to say the least. The most important thing I noticed is that changes for the proxy-to-concrete-model approach will have to be made before Zinnia can upgrade to 1.7. This is because the 1.7 migrations won't even initialize until this issue is first addressed. The concrete model path as I see it: Zinnia will actually have to release two versions, one that will make the DB of those using Zinnia ready for 1.7 and a second that actually supports 1.7. For example, the work involved in each release could be. Final 1.6 release:
1.7 Friendly Release:
A site that uses Zinnia would first have to upgrade to Zinnia's final 1.6 version, applying it's migrations. Then when it's ready to go to 1.7 it would have to upgrade to Zinnia's 1.7 friendly version. Edit: Removed some inaccurate compatibility notes. |
The solution on this issue is ugly, but really simple once you know how the models are historically loaded, and now loaded in Django 1.7 Deferring the loading of the Author models in Then when a components of Zinnia import the Author model, the Author model is registered and applied, like in the older versions of Django. That's why each import of the Author model in Zinnia is done like that:
In this manner the Author model is only loaded when needed, not when the Zinnia's models are imported. No more needs to build a profile models for author, the patch is just a removal of 2 lines. Note the build is broken on Python 3, due to this issue https://code.djangoproject.com/ticket/23455 . |
Tests are now passing. |
I have an error when migrate :
Full log : http://paste.awesom.eu/hN4o (I have edit my Author model with the "patch") |
What is the state of the database when you run the migrate command ? |
The database is empty. Sorry I was using the dev branch and editing the Author model but using the |
Hello All Zinnia Fans! :-)
File "C:\Python27\lib\site-packages\zinnia\models\author.py", line 23, in Regards, K. |
@karolszk There is a feature branch for Django 1.7. |
@bittner thanks, but in branch django1.7 I don't see any changes in zinnia\models\author.py. "Another common culprit is django.contrib.auth.get_user_model(). Use the AUTH_USER_MODEL setting to reference the User model at import time." (https://docs.djangoproject.com/en/1.7/ref/applications/) But I'll try. :-) Regards, K. |
Hi, i have clone Zinnia to my computer(git clone https://github.com/Fantomas42/django-blog-zinnia.git) |
Did you even try searching for it ? |
@Fantomas42, Yet, i searched. i done the follows:
Above steps are OK? Now, when i runserver, then open http://127.0.0.1:8000/zinniablog on browser, but i get these errors:
|
@maniachhz I think it miss the 'django.core.context_processors.request' context processors. Please follow carefully the install instruction. |
@Fantomas42 , |
Hi! if you customize A solution to avoid future conflicts is to create a custom zinnia migration module for zinnia MIGRATION_MODULES = {
'zinnia': 'my_utils.migrations_zinnia'
} and then run a
|
@Fantomas42, are you planning to include this feature branch in the next release of zinnia? If there are issues blocking you from merging this feature branch into master, what needs to be done in order to resolve them? |
Yes the branch is planned to be merged and to be in the next release of Zinnia. Be patient. |
What need to be done in django1.7 feature branch? |
I have a list of things to be done, with 3-4 remaining points, which I can't delegate, mainly documentation and testing new features introduced in Django 1.7, |
Found workaround of issue with ===================================================================
--- zinnia/models/__init__.py (revision a85e910b57a920ae10e5d30be4d32614c8e87268)
+++ zinnia/models/__init__.py (revision )
@@ -1,6 +1,7 @@
"""Models for Zinnia"""
from django_comments.moderation import moderator
+from zinnia.models.author import Author
from zinnia.models.entry import Entry
from zinnia.models.category import Category
@@ -14,6 +15,7 @@
# when the Zinnia's URLs are parsed. Issue #161.
# Issue #161, seems not valid since Django 1.7.
__all__ = [Entry.__name__,
+ Author.__name__,
Category.__name__]
===================================================================
--- zinnia/models/author.py (revision a85e910b57a920ae10e5d30be4d32614c8e87268)
+++ zinnia/models/author.py (revision )
@@ -1,12 +1,16 @@
"""Author model for Zinnia"""
+from django.apps import apps
from django.db import models
-from django.contrib.auth import get_user_model
+from django.conf import settings
from django.utils.encoding import python_2_unicode_compatible
from zinnia.managers import entries_published
from zinnia.managers import EntryRelatedPublishedManager
+user_app, user_model = settings.AUTH_USER_MODEL.split('.')
+
+
class AuthorPublishedManager(models.Model):
"""
Proxy model manager to avoid overriding of
@@ -19,7 +23,7 @@
@python_2_unicode_compatible
-class Author(get_user_model(),
+class Author(apps.get_app_config(user_app).get_model(user_model),
AuthorPublishedManager):
"""
Proxy model around :class:`django.contrib.auth.models.get_user_model`. |
Interesting, tests needed. |
I am getting the same or similar error on python 3.4.0 django 1.7.5.
|
Samer error here with django==1.7.5 and django-blog-zinnia==0.15.1 |
@argaen do you have customized the User model ? |
I have the same error with a customized User model on django==1.7.4 and django.blog.zinnia==0.15.1. |
Oh good, I thought I was the only one there for a moment. @argaen, @aniav What are your user model customizations? (if any) I can't seem to discover what is bugging the migration. |
My user model has no username, only email as a username field. In my case the migration should have been faked as it turned out, because all the tables already existed - but I should now create a separate migration on my side that would change the foreign key constraint for zinnia blog author and set it on the new user model. My application was upgraded from Django 1.4 to 1.7 recently, that's why I had tables created earlier and had to fake migrations. |
@Fantomas42 @faraggi Yeah, I have custom User model. It inherits from |
Me too. From #400 , I added to the zinnia initial migration......
The initial migration now works. |
Having the same problem (with custom user model), and @RossLYoung solution works for me. |
@RossLYoung @argaen @aniav Thanks RossLYoung, your solution also worked for me. |
+1 here. @RossLYoung solution works. |
+1 - @RossLYoung solution worked for me. My workaround: created a copy of the migration, made @RossLYoung change and an entry in MIGRATION_MODULES like 'zinnia': 'ext.zinnia_migrations' |
Hi, This is the output i get when trying to migrate: |
zinnia.models.author.Author uses get_user_model() at module import time, which is forbidden by Django 1.7 (https://docs.djangoproject.com/en/dev/releases/1.7/#app-loading-refactor). 1.7 is due for release soon (beta has been released), so a way to avoid this import time dependency should be figured out.
The text was updated successfully, but these errors were encountered: