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

Migrations are added when social account providers are added or removed #2652

Closed
johanneswilm opened this issue Aug 31, 2020 · 13 comments
Closed

Comments

@johanneswilm
Copy link

Hey,
using Django 3.1, when I add or remove an entry like

"allauth.socialaccount.providers.google",

in INSTALLED_APPS, Django notifies me that I need to run ./manage.py makemigrations and then creates migrations inside of the socialaccount app. The reason for this seems to be that the way the model is written, the list of possible choices is calculated rather than handing over a callable.

This:

provider = models.CharField(verbose_name=_('provider'),
                                max_length=30,
                                choices=providers.registry.as_choices())

Rather than:

provider = models.CharField(verbose_name=_('provider'),
                                max_length=30,
                                choices=providers.registry.as_choices)

I see this has been an issue for a long time and some seem to be able to get around it, but I haven't been able to figure out exactly how they do it.

Any idea why it's like this and what is the best workaround?

@iarp
Copy link
Contributor

iarp commented Aug 31, 2020

What version of allauth? Do you have any allauth based settings set?

@johanneswilm
Copy link
Author

django-allauth==0.41.0

########## ALLAUTH CONFIGURATION
ACCOUNT_EMAIL_REQUIRED = True
ACCOUNT_EMAIL_VERIFICATION = "mandatory"
ACCOUNT_UNIQUE_EMAIL = True
ACCOUNT_FORMS = {
    "signup": "common.forms.CustomSignupForm",
}

########## END ALLAUTH CONFIGURATION

@iarp
Copy link
Contributor

iarp commented Aug 31, 2020

Have you tried reinstalling django-allauth? I wonder if a migration was made accidentally at some point and now its causing the trigger. Essentially you should only have these exact four files in the socialaccount migrations.

@johanneswilm
Copy link
Author

Hey @iarp That is true. I have those four files. But Django notifies me that something is missing and when I run ./manage.py makemigrations it creates a migration 0004 for socialaccount. This won't work as there might at some time be a migration 0004 that is provided from upstream.

As for reinstalling: The problem is I am on a big project where there are many installations running on other machines run by other devs and servers and such. I cannot just uninstall and reinstall on all these other machines as I don't control them.

As far as I can tell, the problem is that the way the model is written and also the migration, means that the list of providers will be taken by executing providers.registry.as_choices() the first time the migration is run. When then later the list of providers changes, Django will add more migrations to socialaccount if you run ./migrate makemigrations.

To be entirely honest, I also use django allauth on a different project where I have changed the list of providers over time, and it has for some reason not caused any migrations to be created. I cannot figure out why that is.

@iarp
Copy link
Contributor

iarp commented Aug 31, 2020

What version of python?

You can't just remove the brackets because django requires choices to be an iterable and not a callable. as_choices() yields from a dict so I have to think that perhaps the order of the dict is not maintaining between comparison.

@johanneswilm
Copy link
Author

Python 3.8.3.

The change I am making is that previously none of the socialaccount providers were enabled. Now Google is enabled.

So the migration Django wants to autocreate looks like this:

# Generated by Django 3.1 on 2020-08-31 13:07

from django.db import migrations, models


class Migration(migrations.Migration):

    dependencies = [
        ('socialaccount', '0003_extra_data_default_dict'),
    ]

    operations = [
        migrations.AlterField(
            model_name='socialaccount',
            name='provider',
            field=models.CharField(choices=[('google', 'Google')], max_length=30, verbose_name='provider'),
        ),
        migrations.AlterField(
            model_name='socialapp',
            name='provider',
            field=models.CharField(choices=[('google', 'Google')], max_length=30, verbose_name='provider'),
        ),
    ]

That seems to be entirely reasonable.

@johanneswilm
Copy link
Author

You can't just remove the brackets because django requires choices to be an iterable and not a callable.

Ah I see. My bad.

I wonder if storing this in the model is the right place then. Maybe it would be better to enforce it at the level of the form that is being shown on the admin page?

If you run a site for several years, I think it's fairly common that your list of connected social sites will change over time. Some sites close down, others are getting more important, some change focus, etc. . So it's probably not a good idea to have to lock in on a list right at the beginning of starting a site ro system of sites, right?

I am wondering what others have been doing to avoid this problem.

@iarp
Copy link
Contributor

iarp commented Aug 31, 2020

That's the thing though, there is no locked in list. Since allauth uses as_choices() its building the list of choices for every migration file when django checks over the migration files for changes.

If you're up to it you could zip up your allauth folder and share it, i'll look it over and see if i can see anything.

Beyond that though I'm not sure. I matched every version of django, allauth, and python that you supplied and i can add/remove providers at will and makemigrations never has a problem.

@johanneswilm
Copy link
Author

That's the thing though, there is no locked in list. Since allauth uses as_choices() its building the list of choices for every migration file when django checks over the migration files for changes.

I see. Maybe there have been some changes in how Django does that? Or maybe some changes per database? I am using postgresql, if that makes a difference. I have seen the issue has come up before [1], so maybe relying on it not actually storing the choices anywhere doesn't work under some specific setups?

If you're up to it you could zip up your allauth folder and share it, i'll look it over and see if i can see anything.

I don't have a special allauth folder that is different from what you people distribute. I just install allauth using:

pip install django-allauth

So it tries to create a migration file in that place: /home/johannes/.pyenv/versions/3.8.3/lib/python3.8/site-packages/allauth/socialaccount/migrations/0004_blablabla.py

[1] #749

@iarp
Copy link
Contributor

iarp commented Aug 31, 2020

Migrations do not depend on the database, they are based on the existing migration files and the current models. The model and the initial migration are based on as_choices(). As long as the output of as_choices() at the time makemigrations is called is the exact same in both model and migration file then there is no change detected regardless of adding or removing a provider.

Somehow either one of two things has happened:

There is also a possibility of some sort of obscure issue is changing how providers are being loaded between model initialization and migration checks. For instance if I put print(provider_cls.id, provider_cls.name) just above the yield in the as_choices() method, When I run makemigrations while having google as the only provider, i see google printed 4 times. Once for SocialApp class initialization and again for SocialAccount class initialization, then two more times for the migration of SocialApp and SocialAccount. If you did the same and don't see 4 then something is wrong.

@johanneswilm
Copy link
Author

Ah, thanks! I found it. The iwas apparently that I changed the name of the Google Provider in the top level urls.py file:

from allauth.socialaccount.providers.google.provider import GoogleProvider

GoogleProvider.name = "Alphabet"  # Changes name of google button on login page.

This change was then not applied during the initial migration but was applied when checking the model. I have entirely removed this now and just put it in the template instead.

Thanks so much, @iarp for helping with this!

@iarp
Copy link
Contributor

iarp commented Aug 31, 2020

Out of curiosity how did you find that as being the issue ?

@johanneswilm
Copy link
Author

I added the print line you mentioned and the output was:

google Google
google Google
google Alphabet
google Alphabet

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

No branches or pull requests

2 participants