procrastinate icon indicating copy to clipboard operation
procrastinate copied to clipboard

Admin page support for large args

Open jakajancar opened this issue 10 months ago • 16 comments

We have somewhat large args (megabytes) and procrastinate admin page is not made for this. Would it be possible to add an ellipsis after say 100 chars of an arg? Can this be overridden somehow?

jakajancar avatar Jan 30 '25 10:01 jakajancar

isn't that already the case ? https://github.com/procrastinate-org/procrastinate/blob/0ced689659fd42b93421776b8c106e5e2f46d55e/procrastinate/contrib/django/admin.py#L98-L99

ewjoachim avatar Jan 30 '25 12:01 ewjoachim

  • If it doesn't work, then we need to fix it
  • If you'd like to change the constant: it makes sense we could do a Django setting for that.

Either way: would you be interested to contribute that in a PR ? Settings are:

ewjoachim avatar Jan 30 '25 12:01 ewjoachim

Oops, my bad, it indeed does work 🙊

However, even with 2000 lines, if there is a bunch of args I guess that's fine, but with a single 1000 char one, it stretches the table column and it's impossible to see the status. Any thoughts on clipping individual args instead?

//edit: or maybe simpler to implement: maximum string length

jakajancar avatar Jan 30 '25 14:01 jakajancar

I would like to help with this one

I guess sometimes, especially if args are pretty big, it would be useful to disable showing args or even disable querying them from a database

Simple temporary solution before adding new settings I see now: inherit from base admin class in your custom app and override anything you want


As permanent solution:

  • Add PROCRASTINATE_ADMIN_MAX_LEN_ARGS;
  • Add PROCRASTINATE_ADMIN_HIDE_ARGS.

Or add PROCRASTINATE_ADMIN_MAX_LEN_ARGS only and interpret 0 as do not query at all

Is it okey?

Also I was wondering is it safe to filter by task_name (sources)? As far as I see task name has no index. May be it's not a problem since developer should clean old tasks but I don't know...

denisSurkov avatar Feb 03 '25 16:02 denisSurkov

PROCRASTINATE_ADMIN_MAX_LEN_ARGS

Yes but depending on how nested it is, it might still overflow. It's as much about length as it's about width. We could also use a special json encoder that would truncate strings and lists (maybe that would be 2 more settings)

Alternatively: make it extra-easy to implement one's own Admin subclass and do whatever they want. Maybe give a few examples? as mentioned below.)

I think I'd indeed rather we create extendable building blocks that users can customize to their liking than an approach when there's a knob for everything.

If we do that, I guess we probably don't want this on the detail view, though we may want overflow: scroll if we can. I think if we look at the details of a task, chances are we want the whole thing.

PROCRASTINATE_ADMIN_HIDE_ARGS

I guess it might be interesting in some cases, but I don't like negative settings. Let's use PROCRASTINATE_ADMIN_SHOW_ARGS instead. But... I believe we should be able to do that with inheritance too (though it might get annoying because we'll have to actively remove something our base class did). Probably a choice to make while implementing :)

(Also, good suggestion for inheriting the base admin class, it may be worth mentioning this in the doc's how-to section)

If you want to make a PR, that's very good news ! It will be appreciated!

ewjoachim avatar Feb 03 '25 17:02 ewjoachim

In my original case, I would actually have preferred to just hide one of the arguments for a specific job type, but didn't think of customization at this level as an option.

Indeed that would be the best, to allow me a function that processes the args arbitrarily.

jakajancar avatar Feb 03 '25 18:02 jakajancar

I think it's possible to do it without any modifications:

# your_custom_app/admin.py

from django.contrib import admin
from procrastinate.contrib.django.admin import ProcrastinateJobAdmin
from procrastinate.contrib.django.models import ProcrastinateJob

admin.site.unregister(ProcrastinateJob)

@admin.register(ProcrastinateJob)
class MySuperAdmin(ProcrastinateJobAdmin):
    list_display = [
        "pk",
        "short_task_name",
        # for example, no "pretty_args",
        "pretty_status",
        "summary",
    ]

I tried:

INSTALLED_APPS = [
   ...
    "app_one",
    "procrastinate.contrib.django",
]

and

INSTALLED_APPS = [
   ...
    "procrastinate.contrib.django",
    "app_one",
]

And it's working fine.

@jakajancar , can you check if it solves your problem?

denisSurkov avatar Feb 04 '25 15:02 denisSurkov

But I want args (just task name is kinda useless), just clipped/transformed differently, can I do that? (Sorry, Django noob)

jakajancar avatar Feb 04 '25 17:02 jakajancar

Yes, no problem

# your_custom_app/admin.py

from django.contrib import admin
from procrastinate.contrib.django.admin import ProcrastinateJobAdmin
from procrastinate.contrib.django.models import ProcrastinateJob

admin.site.unregister(ProcrastinateJob)

@admin.register(ProcrastinateJob)
class MySuperAdmin(ProcrastinateJobAdmin):
    list_display = [
        "pk",
        "short_task_name",
        "pretty_args",
        "pretty_status",
        "summary",
    ]

    @admin.display(description="Args")
    def pretty_args(self, instance: models.ProcrastinateJob) -> str:
        # here you can do whatever you want

        important_arg = str(instance.args['path_to_your_argument'])
       
        return format_html(
            '<pre style="margin: 0">{important_arg}</pre>', important_arg=important_arg
        )

I think this should work. If something does not work as expected fell free to ask ;)

denisSurkov avatar Feb 04 '25 17:02 denisSurkov

Then it's ok to settle on just adding this to the documentation :)

Also: do anyone think the current value is not a suitable default for usual users ?

ewjoachim avatar Feb 04 '25 18:02 ewjoachim

Hey, coming back to this now. I don't think just fixing pretty_args is enough, the title and logging will still be a problem if you have a arguments that go into a megabyte. I think Job.call_string would need to have some limiting or be customizable too. What are your thoughts?

For context, we're ingesting emails and just dumping the EML into an argument. It's simpler than having to create and manually manage a related entity.

I thought about allowing customization of Job.call_string, but I'm not sure how to get any settings in there.

Then I thought about using a custom class and defining a custom shorter __repr__. Looking at the code, not sure you can override json_dumps and json_loads when using the Django integration. Also I think the logged/displayed values are post-serialization.

jakajancar avatar May 04 '25 22:05 jakajancar

@ewjoachim If you think being able to support larger args is valuable and can point me into the right direction soon-ish, I can dedicate some time this week to prepare a PR, it became pretty urgent for us to fix. Otherwise I'll go with an added level of indirection and store the data in a separate table outside the kwarg.

jakajancar avatar May 05 '25 05:05 jakajancar

(placeholder: I intend to answer this today, but no time right now)

ewjoachim avatar May 05 '25 13:05 ewjoachim

Ah yeah the call string.

Indeed, it would probably make sense to be able to keep only the beginning and the end of any arg more than, say, 100 chars.

We'll probably need that to be effective in multiple place, so it's possible we'll need the configuration for this to be on the app, or the task. I'd be tempted to imagine a sane default that 99% of users will use, and a powerful override, where power users might completely change the call string to fit their requirements.

Let’s start with: add an optional property when creating the app that lets people provide a callable that receives a job and returns its string description

Let’s also make it so that by default, argument values are truncated over 100 chars.

Let’s document this.

I think if we do that, people will be autonomous to find what suits them best ?

ewjoachim avatar May 05 '25 15:05 ewjoachim

Thank you @ewjoachim, I will work on this!

jakajancar avatar May 06 '25 05:05 jakajancar

Some ideas @ewjoachim and I had after some discussion (so that we don't forget about it):

  • In general, when a custom encoder is set, encode the args with the custom encoder and decode with the default decoder (the result is valid JSON then)
  • In the admin interface:
    • Shorten long args with a custom algorithm (output must not be valid JSON)
    • Provide a way to present the full (unshortened) args (valid JSON)
  • In the logs:
    • Have an environment variable if long args should be shortened or not
    • If args should be shortened, then log the args as a shortened string
    • If args should not be shortened, then log the full args as valid JSON

@ewjoachim Feel free to edit this comment if I have reproduced something wrong.

medihack avatar May 18 '25 11:05 medihack