all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Hannes Laimer <h.laimer@proxmox.com>, pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [PATCH pmg-api v2 1/1] fix #3450: api: queue: add POST endpoint for batch deletion/delivery
Date: Wed, 24 Sep 2025 10:19:53 +0200	[thread overview]
Message-ID: <3b4256c7-93f7-47fc-a084-bae5a7ac9ab6@proxmox.com> (raw)
In-Reply-To: <7104c7f5-da14-43f1-a0b4-6507ae1f9e46@proxmox.com>

Am 24.09.25 um 07:43 schrieb Hannes Laimer:
> On 23.09.25 15:22, Thomas Lamprecht wrote:
>> Am 23.09.25 um 11:33 schrieb Hannes Laimer:

>>> - single POST endpoitn that takes type(delete/deliver) and list of id's
>>
>> Fine for delivery, but why not add a new optional ids parameter to the
>> existing DELETE endpoint?
>>
> 
> To have it mentioned here, the reason is that
> DELETE doesn't take data, at least our proxy doesn't, and something like
> `.../{queue}/?ids=AABC234,DDEF4543,EEFG6473,...`
> is at best odd and brittle.

Meh, nothing should speak against DELETE taking a body from a technical
POV, but fair enough.

> I should have included this and I'll add it for v3.

Yes, please do!

>>> diff --git a/src/PMG/API2/Postfix.pm b/src/PMG/API2/Postfix.pm
>>> index ba0689c..90a74e9 100644
>>> --- a/src/PMG/API2/Postfix.pm
>>> +++ b/src/PMG/API2/Postfix.pm
>>> @@ -335,6 +335,49 @@ __PACKAGE__->register_method({
>>>       },
>>>   });
>>>   +__PACKAGE__->register_method({
>>> +    name => 'queue_action',
>>> +    path => 'queue/{queue}',
>>> +    method => 'POST',
>>> +    description => "Perform an action on the given queue IDs (delete/deliver).",
>>> +    proxyto => 'node',
>>> +    permissions => { check => ['admin'] },
>>> +    protected => 1,
>>> +    parameters => {
>>> +        additionalProperties => 0,
>>> +        properties => {
>>> +            node => get_standard_option('pve-node'),
>>> +            queue => $queue_name_option,
>>> +            action => {
>>> +                description => 'Operation to perform on the given IDs.',
>>> +                type => 'string',
>>> +                enum => ['delete', 'deliver'],
>>> +            },
>>> +            id => {
>>
>> "ids" would slightly better signal that this can be more than one.
>>
>>> +                description => 'Queue ID(s), separated by semicolons (;).',
>>> +                type => 'string',
>>> +                pattern => '[A-Za-z0-9]{8,20}(;[A-Za-z0-9]{8,20})*',
>>
>> hmm, don't we have a format for this or at least an existing regex
>> we can reuse?
>>
> 
> not really, there's only
> ```
> my $queue_id_option = {
>     description => "The Message queue ID.",
>     type => 'string',
>     pattern => '[a-zA-Z0-9]+',
>     minLength => 8,
>     maxLength => 20,
> };
> ```
> but I don't think a thing for just `[a-zA-Z0-9]+` adds much.

it's obviously trivial and indeed does not add much on it's own
from a low-level POV, but these can be abstractions that still help
to argue and reason about the code nonetheless; that said, I'm fine
with deciding some not so hard cuts on a case by case basis, and for
me it can be fine to keep this as is then if there is nothing factored
out already.





_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel


  reply	other threads:[~2025-09-24  8:19 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-23  9:33 [pmg-devel] [PATCH pmg-api/pmg-gui v2 0/2] fix #3450: batch deletion/delivery for postfix queue Hannes Laimer
2025-09-23  9:33 ` [pmg-devel] [PATCH pmg-api v2 1/1] fix #3450: api: queue: add POST endpoint for batch deletion/delivery Hannes Laimer
2025-09-23 13:22   ` Thomas Lamprecht
2025-09-24  5:43     ` Hannes Laimer
2025-09-24  8:19       ` Thomas Lamprecht [this message]
2025-09-23  9:33 ` [pmg-devel] [PATCH pmg-gui v2 1/1] fix #3450: ui: queue: multi-select for item deletion/delivery Hannes Laimer
2025-09-23 13:27   ` Thomas Lamprecht
2025-09-24  7:14 ` [pmg-devel] superseded: [PATCH pmg-api/pmg-gui v2 0/2] fix #3450: batch deletion/delivery for postfix queue Hannes Laimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3b4256c7-93f7-47fc-a084-bae5a7ac9ab6@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=h.laimer@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal