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

Fix #133 -- Make sure initialization only happens once #134

Closed
wants to merge 0 commits into from

Conversation

huubbouma
Copy link

This fixes the issue I encounter. The fix is pretty simple, using a global variable. GLobal variables are in use anyway, so it seemed legit to use it here as well

Comment on lines 49 to 52
def ready(self):
if getattr(self, "_is_ready", False):
return
super().ready()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having this piece in place would mean that we will never call ready() method, as here initialize is setting _is_ready=True. This generally won't be correct behaviour, even if it may solve the issue with multiple ready() call.

Would you mind trying out #137 this solution to see if this changes things for you?

Best,
Rust

Copy link
Author

Choose a reason for hiding this comment

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

#137 works perfectly for me! So please ignore this PR and go ahead with that one, which is much better :-)

Comment on lines 40 to 49
message_dict = instance.message._asdict()

# make sure we can still get a representation of the
# kwargs payload when a non json encoder is in use
kwargs_encoder = DjangoDramatiqConfig.select_encoder()
if not isinstance(kwargs_encoder, JSONEncoder):
for k,v in message_dict['kwargs'].items():
message_dict['kwargs'][k] = f"<{v}>"

message_details = json.dumps(message_dict, indent=4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems like a code that solves a different issue rather than #133. Was the second commit pushed by a mistake? It would be good to address different issues separately. 🙏

Copy link
Author

Choose a reason for hiding this comment

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

yes you're right. I've made a mistake with the push to the main branch

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