From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 1C3A27442F
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Jun 2021 16:33:21 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 0CC6B2AF02
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Jun 2021 16:33:21 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 692E02AEF4
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Jun 2021 16:33:20 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 318664363D
 for <pbs-devel@lists.proxmox.com>; Tue,  1 Jun 2021 16:33:20 +0200 (CEST)
Message-ID: <5876c1ef-7a68-feb2-36ce-06ef904e3032@proxmox.com>
Date: Tue, 1 Jun 2021 16:33:19 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101
 Thunderbird/89.0
Content-Language: en-US
To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>, Dominik Csapak <d.csapak@proxmox.com>
References: <20210601121350.19919-1-d.csapak@proxmox.com>
 <20210601121350.19919-3-d.csapak@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
In-Reply-To: <20210601121350.19919-3-d.csapak@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.323 Adjusted score from AWL reputation of From: address
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 NICE_REPLY_A           -0.613 Looks like a legit reply (A)
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [datastore.rs]
Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/4] api2/confif/datastore:
 add create datastore api call in a worker
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 01 Jun 2021 14:33:21 -0000

On 01.06.21 14:13, Dominik Csapak wrote:
> so that longer running creates (e.g. a slow storage), does not
> run in a timeout and we can follow its creation
> 

In PVE we did not treated a changed in return value as breaking change, I mean not
in general, as it is a bit depended on what was returned previously, but we didn't
considered it if there was nothing returned before.

I'd rather do one either:
* wait for the 2.0 branch and just move over the existing one to always return a task
  UPID, least amount of work and as 2.0 is not that far away it would be quite reasonable
  IMO

* just switch now, the manager CLI can be adapted in one go, and the gui can switch too
  as both are shipped by the exact same binary package, also OK IMO, but others may be
  have objections (?)

* return the upid as option and add an opt-in switch for the in worker, and if not set
  then poll in sync for it being finished, could be removed with 2.0 or just the default
  switched

mostly as I'd like to avoid duplicate entries of endpoints where we plan to use the new
one only anyway (as quite some metadata IO is involved it's effectively the only sensible
one).

> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/api2/config/datastore.rs | 107 ++++++++++++++++++++++++++++++++++-
>  www/window/DataStoreEdit.js  |   3 +-
>  2 files changed, 108 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
> index 33e76b27..16ac030f 100644
> --- a/src/api2/config/datastore.rs
> +++ b/src/api2/config/datastore.rs
> @@ -13,7 +13,7 @@ use crate::backup::*;
>  use crate::config::cached_user_info::CachedUserInfo;
>  use crate::config::datastore::{self, DataStoreConfig, DIR_NAME_SCHEMA};
>  use crate::config::acl::{PRIV_DATASTORE_ALLOCATE, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY};
> -use crate::server::jobstate;
> +use crate::server::{jobstate, WorkerTask};
>  
>  #[api(
>      input: {
> @@ -143,6 +143,110 @@ pub fn create_datastore(param: Value) -> Result<(), Error> {
>      Ok(())
>  }
>  
> +#[api(
> +    protected: true,
> +    input: {
> +        properties: {
> +            name: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +            path: {
> +                schema: DIR_NAME_SCHEMA,
> +            },
> +            comment: {
> +                optional: true,
> +                schema: SINGLE_LINE_COMMENT_SCHEMA,
> +            },
> +            "notify-user": {
> +                optional: true,
> +                type: Userid,
> +            },
> +            "notify": {
> +                optional: true,
> +                schema: DATASTORE_NOTIFY_STRING_SCHEMA,
> +            },
> +            "gc-schedule": {
> +                optional: true,
> +                schema: GC_SCHEDULE_SCHEMA,
> +            },
> +            "prune-schedule": {
> +                optional: true,
> +                schema: PRUNE_SCHEDULE_SCHEMA,
> +            },
> +            "keep-last": {
> +                optional: true,
> +                schema: PRUNE_SCHEMA_KEEP_LAST,
> +            },
> +            "keep-hourly": {
> +                optional: true,
> +                schema: PRUNE_SCHEMA_KEEP_HOURLY,
> +            },
> +            "keep-daily": {
> +                optional: true,
> +                schema: PRUNE_SCHEMA_KEEP_DAILY,
> +            },
> +            "keep-weekly": {
> +                optional: true,
> +                schema: PRUNE_SCHEMA_KEEP_WEEKLY,
> +            },
> +            "keep-monthly": {
> +                optional: true,
> +                schema: PRUNE_SCHEMA_KEEP_MONTHLY,
> +            },
> +            "keep-yearly": {
> +                optional: true,
> +                schema: PRUNE_SCHEMA_KEEP_YEARLY,
> +            },
> +        },
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false),
> +    },
> +)]
> +/// Create new datastore config.
> +pub fn create_datastore_worker(
> +    param: Value,

for new API endpoints, even if copied, it'd be great to avoid `param: Value`