Queue Depth Utils #33
Conversation
django_dbq/test_task.py
Outdated
| from django_dbq.models import Job | ||
|
|
||
|
|
||
| def test_task(job): |
There was a problem hiding this comment.
that wasn't supposed to reach github :p
django_dbq/models.py
Outdated
| annotation_dicts = ( | ||
| Job.objects.filter(state__in=(Job.STATES.READY, Job.STATES.NEW)) | ||
| .values("queue_name") | ||
| .order_by() |
There was a problem hiding this comment.
Is this line necessary? I don't think the order really matters here?
|
Is adding an implementation of #26 in scope for this PR? |
|
i don't see why not @RealOrangeOne -- that's where the idea came from in the first place :D |
django_dbq/models.py
Outdated
| @@ -137,3 +137,17 @@ def run_creation_hook(self): | |||
| logger.info("Running creation hook %s for new job", creation_hook_name) | |||
| creation_hook_function = import_string(creation_hook_name) | |||
| creation_hook_function(self) | |||
|
|
|||
| @staticmethod | |||
| def get_queue_depths_dict(): | |||
There was a problem hiding this comment.
Thoughts on whether this needs the dict suffix? Putting the type in the method name feels weird
| def get_queue_depths_dict(): | |
| def get_queue_depths(): |
There was a problem hiding this comment.
yeah that's a fair point! i'll update that
Co-Authored-By: Jake Howard <RealOrangeOne@users.noreply.github.com>
Co-Authored-By: Jake Howard <RealOrangeOne@users.noreply.github.com>
|
|
||
| def handle(self, *args, **options): | ||
| queue_name = options["queue_name"] | ||
| queue_names = options["queue_name"] |
There was a problem hiding this comment.
Should we rename the argument to queue_names too for consistency?
There was a problem hiding this comment.
so when i run manage.py queue_depth --help it outputs the following:
usage: manage.py queue_depth [-h] [--version] [-v {0,1,2,3}]
[--settings SETTINGS] [--pythonpath PYTHONPATH]
[--traceback] [--no-color]
[queue_name [queue_name ...]]
it would maybe look a little weird if that said queue_names instead?
There was a problem hiding this comment.
Ah ok. Yes I do think it reads better like that, actually.
| [ | ||
| f"{queue_name}={queue_depths.get(queue_name, 0)}" | ||
| for queue_name in queue_names | ||
| ] |
There was a problem hiding this comment.
I feel like if we're going to use this for cloudwatch metrics, we'll need a way to identify these log lines more unambiguously. Maybe write out something like "Queue depths: default=0" or event=queue_depths: default=0 or similar? (the latter would be better for us but maybe less "friendly"? Not sure..)
There was a problem hiding this comment.
i think the latter is probably better for everyone, it's still human readable and more easily machine readable than the former
…e into queue-depth-utils
Job.get_queue_depths_dictmethodmanage.py queue_depthcommand from Add "manage.py queue_depth" command #26