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 information about queue and priority #345

Merged
merged 5 commits into from
Oct 19, 2023
Merged

Add information about queue and priority #345

merged 5 commits into from
Oct 19, 2023

Conversation

jrobichaud
Copy link
Owner

@jrobichaud jrobichaud commented Oct 16, 2023

Todo:

  • add task_routing_key and task_properties to modify_context_before_task_publish
  • refactor celery receivers and command receivers modules to use classes
  • (maybe?) log queue/routing_key and priority in task_enqueued
  • documentation

Closes #341

@jrobichaud jrobichaud marked this pull request as draft October 16, 2023 02:08
@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (4b82f27) 100.00% compared to head (d87db34) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #345   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           32        32           
  Lines          459       459           
  Branches        23        23           
=========================================
  Hits           459       459           
Files Coverage Δ
django_structlog_demo_project/home/views.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mohamedTbarka
Copy link

I am wonderin' why did u choose to use classes, including classes' members, at the level of the receivers ?

@jrobichaud
Copy link
Owner Author

jrobichaud commented Oct 18, 2023

@mohamedTbarka it is not finished but I will have to store data somewhere between "receiver_before_task_publish" and "receiver_after_task_publish"

the value of priority is available in the first but I want to use it in the second.

I did not want to store the value in a global variable.

Do you think this is a bad idea?

@mohamedTbarka
Copy link

@jrobichaud, I think it's slickly done; I needed just a clarification 🙏

@jrobichaud
Copy link
Owner Author

@mohamedTbarka

I discovered two things about signals:

  1. the connected functions need to continue to exist or they get disconnected. (ex: if the function is declared and connected in a function, it will be disconnected when exiting the function). This is why i keep a reference of the instance in the app or the bootstep.
  2. the receivers can actually be methods (I discovered that yesterday). 🤯

@mohamedTbarka
Copy link

@jrobichaud, thank u a lot for the clarifications 🙏. I think I need to play more with the signals to grasp why such behavior exists 🤔

@jrobichaud
Copy link
Owner Author

Since it seems like people are watching me working on this, what is your opinion about logging or not routing_key?

@badziyoussef
Copy link
Contributor

Since it seems like people are watching me working on this, what is your opinion about logging or not routing_key?

I would say Yes, because sometimes, task is enqueued and queues are full, I would like to know the content of any desired queue and I can do that if I have routing_key part of the log task_enqueued

@jrobichaud
Copy link
Owner Author

@badziyoussef thanks for the concrete example.

Anything else from before_task_publish or after_task_publish would be useful to be added to the task_enqueued event?

@jrobichaud jrobichaud marked this pull request as ready for review October 18, 2023 23:59
@jrobichaud jrobichaud merged commit 5dba808 into main Oct 19, 2023
12 checks passed
@jrobichaud jrobichaud deleted the queue-info branch October 19, 2023 00:04
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.

Add queue information to task_enqueued log
3 participants