Re: [pbs-devel] [PATCH proxmox-backup 2/4] api2/confif/datastore: add create datastore api call in a worker

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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`