From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Dominik Csapak <d.csapak@proxmox.com>
Cc: pve-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com
Subject: Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler
Date: Tue, 5 Jul 2022 13:39:41 +0200 [thread overview]
Message-ID: <20220705113941.kj4qawt6v63owiqm@casey.proxmox.com> (raw)
In-Reply-To: <20220531111726.2972022-4-d.csapak@proxmox.com>
On Tue, May 31, 2022 at 01:17:22PM +0200, Dominik Csapak wrote:
> this can be 'plain', 'pxar', 'zip' or 'tar.zst', and it returns the
> content in the given format (with fallback to the old behaviour if not
> given)
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> note: needs a bumped 'proxmox-compression' in the Cargo.toml to build
>
> .../src/proxmox_restore_daemon/api.rs | 49 ++++++++++++++++---
> 1 file changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
> index aeb5a71d..c4977ce6 100644
> --- a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
> +++ b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs
> @@ -13,7 +13,7 @@ use serde_json::Value;
> use tokio::sync::Semaphore;
>
> use pathpatterns::{MatchEntry, MatchPattern, MatchType, Pattern};
> -use proxmox_compression::zip::zip_directory;
> +use proxmox_compression::{tar::tar_directory, zip::zip_directory, zstd::ZstdEncoder};
> use proxmox_router::{
> list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, Router,
> RpcEnvironment, SubdirMap,
> @@ -236,11 +236,24 @@ pub const API_METHOD_EXTRACT: ApiMethod = ApiMethod::new(
> true,
> &BooleanSchema::new(concat!(
> "if true, return a pxar archive, otherwise either the ",
> - "file content or the directory as a zip file"
> + "file content or the directory as a zip file. DEPRECATED: use 'format' instead."
> ))
> .default(true)
> .schema()
> - )
> + ),
> + (
> + "format",
> + true,
> + &StringSchema::new("The desired format of the result.")
> + .format(&ApiStringFormat::Enum(&[
> + EnumEntry::new("plain", "Plain file (only works for single files)"),
> + EnumEntry::new("pxar", "PXAR archive"),
> + EnumEntry::new("zip", "ZIP archive"),
> + EnumEntry::new("tar.zst", "Zstd compressed TAR archive"),
> + ]))
> + .default("pxar")
> + .schema()
If you make an `#[api] enum RestoreFormat {}` and `use proxmox_schema::ApiType` you
can pass the enum's `RestoreFormat::API_SCHEMA` in here.
Would be nicer than using strings... (you can use `#[serde(rename)]` for
the "tar.zst" value).
On the other hand... We actually use a *tar* encoder, not a *tar.zstd*
encoder (obviously, given the nature of how this works), which makes me
wonder, shouldn't `zstd` maybe be a separate boolean?
I do think for a large single file "plain+zstd" makes sense, also
zstd-compressing a pxar might be worthwhile...
(And while zip+zstd doesn't make *that* much sense... I wouldn't really
care much either ;-) )
> + ),
> ]),
> ),
> )
> @@ -271,6 +284,11 @@ fn extract(
> let path = Path::new(OsStr::from_bytes(&path[..]));
>
> let pxar = param["pxar"].as_bool().unwrap_or(true);
> + let format = match param["format"].as_str() {
> + Some(format) => format.to_string(),
Maybe consider erroring if both `pxar` and `format` are set here?
Otherwise move the pxar binding into the None scope so it's gone
afterwards, it's a long function ;-)
> + // FIXME: old default was plain or zip, remove that with 3.0
> + None => if pxar { "pxar".to_string() } else { String::new() }
> + };
>
> let query_result = proxmox_async::runtime::block_in_place(move || {
> let mut disk_state = crate::DISK_STATE.lock().unwrap();
next prev parent reply other threads:[~2022-07-05 11:40 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-31 11:17 [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox 1/2] proxmox-compression: make ZstdEncoder stream a bit more generic Dominik Csapak
2022-07-05 11:47 ` [pve-devel] applied-both: " Wolfgang Bumiller
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox 2/2] proxmox-compression: add 'tar_directory' Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler Dominik Csapak
2022-07-05 11:39 ` Wolfgang Bumiller [this message]
2022-05-31 11:17 ` [pve-devel] [PATCH proxmox-backup 2/2] file-restore: add 'tar' option to 'extract' command Dominik Csapak
2022-07-05 11:43 ` [pve-devel] [pbs-devel] " Wolfgang Bumiller
2022-05-31 11:17 ` [pve-devel] [PATCH common 1/1] PBSClient: add 'tar' parameter to file_restore_extract Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH storage 1/1] api/filerestore: add 'tar' parameter to 'download' api Dominik Csapak
2022-05-31 11:17 ` [pve-devel] [PATCH widget-toolkit 1/1] window/FileBrowser: enable tar button by default Dominik Csapak
2022-07-01 12:12 ` [pve-devel] [PATCH proxmox/backup/common/storage/wt] add tar.zst download in pve Dominik Csapak
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=20220705113941.kj4qawt6v63owiqm@casey.proxmox.com \
--to=w.bumiller@proxmox.com \
--cc=d.csapak@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=pve-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox