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

Migration created after makemigrations with DEFAULT_AUTO_FIELD modified settings #684

Closed
ventur40 opened this issue Aug 11, 2022 · 10 comments

Comments

@ventur40
Copy link

An migration called "0002_alter_rate_id.py" created after running "makemigrations" with
DEFAULT_AUTO_FIELD = 'django.db.models.BigAutoField'

@benjaoming
Copy link
Contributor

Thanks for the report! Can you describe how you would propose to solve this?

@ventur40
Copy link
Author

I have done a temporal workaround with a new app config:

class ModifiedDjmoneyContribExchangeConfig(AppConfig):
       name = 'djmoney.contrib.exchange'
       default_auto_field = 'django.db.models.AutoField'

And replacing the installed_apps line

    INSTALLED_APPS = [
    ...
    # replace with 'djmoney.contrib.exchange',
    '<path_to_file>.apps.ModifiedDjmoneyContribExchangeConfig',
    ]

@benjaoming
Copy link
Contributor

I see now! I wondered what the issue was since djmoney does specify its default_auto_field.

There's an App Config missing here:

https://github.com/django-money/django-money/tree/main/djmoney/contrib/exchange

PR is welcome 👍

Btw. do you need BigAutoField because of the models in djmoney.contrib.exchange?

@ventur40
Copy link
Author

PR created.

I don't need BigAutoField, it is just a test project.

@ventur40
Copy link
Author

My workaround is not working because of:

    if "djmoney.contrib.exchange" not in settings.INSTALLED_APPS:
        raise ImproperlyConfigured(
            "You have to add 'djmoney.contrib.exchange' to INSTALLED_APPS in order to use currency exchange"
        )

inside convert_money function.

Any idea on how to fix this?

@benjaoming
Copy link
Contributor

Good that you found that! Perhaps the expression can be rewritten? if not any(app.startswith("djmoney.contrib.exchange") for app in settings.INSTALLED_APPS)

@alexnorgaard
Copy link
Contributor

Any news on this? What did you end up with as a workaround? I see the CI checks for the PR were cancelled

@alexnorgaard
Copy link
Contributor

I have made a fix with a different approach here: #716
The change explicitly sets the id field for the Rate model, circumventing the global setting for DEFAULT_AUTO_FIELD

@alexnorgaard
Copy link
Contributor

This issue should probably be closed

@benjaoming
Copy link
Contributor

Thanks for confirming 🙏

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