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

Add new task argument to store metadata #705

Closed
wants to merge 1 commit into from

Conversation

Zhenay
Copy link

@Zhenay Zhenay commented Jan 26, 2023

I wrote integration for sentry, and want to add tracing between scheduling and task execution events. To do this, I need to pass trace_id (generated by sentry-sdk) between them.

The integration wraps some methods transparently for users, so I can put trace_id into args or kwargs.
But it's definitely a bad idea for this purpose.
It will be work fine as long as sentry-sdk is enabled, however a user can disable sentry-sdk between scheduling and task execution. Then kwargs will be contain the unwanted param, which can cause errors.

So I want to add the meta param into the Task and Message classes, which will be used to any metadata of a task at runtime.

@coleifer
Copy link
Owner

This could cause problems for all existing installations of huey, as it changes the serialization format. Going to pass.

@coleifer coleifer closed this Jan 26, 2023
@Zhenay
Copy link
Author

Zhenay commented Jan 27, 2023

Hello, @coleifer.
Thank you for such a quick reaction.

You have made similar changes at least once. But I could find nothing about possible problems in changelog.
What kind of problems will be cause this pr, and why are there no such problems in that case?

@coleifer
Copy link
Owner

Oh I forgot I implemented 6b1a9f9 which makes adding params safe. That said I still think I'll pass for now, but will keep an eye out for other requestors in the future.

@Zhenay
Copy link
Author

Zhenay commented Jan 27, 2023

I agree this case isn't very popular probably, but it may be very helpful for many people in investigating errors and checking the health of systems right now.

So I'll be happy if you change your opinion.

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.

None yet

2 participants