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

Replace AppConfig.ready workaround with __init__ #137

Merged
merged 1 commit into from
Nov 18, 2022
Merged

Conversation

amureki
Copy link
Collaborator

@amureki amureki commented Nov 15, 2022

Attempt to simplify and not break dramatiq configuration logic.
Because AppConfig.ready is being called rather late,
we added workaround to call ready() earlier during model import.
However, this adds complexity and can bring unexpected implications
if someone is overwriting apps.DjangoDramatiqConfig.

Attempt to simplify and not break dramatiq configuration logic.
Because `AppConfig.ready` is being called rather late,
we added workaround to call `ready()` earlier during model import.
However, this adds complexity and can bring unexpected implications
if someone is overwriting `apps.DjangoDramatiqConfig`.
@amureki amureki self-assigned this Nov 15, 2022
@amureki amureki marked this pull request as ready for review November 17, 2022 07:57
@amureki
Copy link
Collaborator Author

amureki commented Nov 17, 2022

@Bogdanp may I ask you to check this one as well? Idea born during the investigation of #133.
I wonder if this was ever a solution before. It is a bit different from DjangoDramatiqConfig.initialize() which we got rid of...

@amureki amureki requested a review from Bogdanp November 17, 2022 07:59
Copy link
Owner

@Bogdanp Bogdanp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I'm not sure I ever tried this, but it does seem like it strikes a balance between the two approaches.

@amureki amureki merged commit 9667f5d into master Nov 18, 2022
@amureki amureki deleted the drop-ready branch November 18, 2022 08:33
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.

2 participants