From: Filip Schauer <f.schauer@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [PATCH vma-to-pbs v4 2/6] add support for bulk import of a dump directory
Date: Mon, 11 Nov 2024 14:13:45 +0100 [thread overview]
Message-ID: <976c12d0-482f-4443-9780-21b02ec50b92@proxmox.com> (raw)
In-Reply-To: <1730724842.agk2is6zq8.astroid@yuna.none>
Superseded by:
https://lists.proxmox.com/pipermail/pbs-devel/2024-November/011353.html
On 04/11/2024 14:06, Fabian Grünbichler wrote:
> grouped_vmas should still be a map, not a vec of vec.. e.g., something
> like this (requires some more adaptation - while this could use
> itertools, I don't think it's worth to pull that in if the same can be
> had with a single fold invocation): @@ -298,12 +298,16 @@ fn
> parse_args() -> Result<BackupVmaToPbsArgs, Error> {
> vmas.sort_by_key(|d|d.backup_time); let total_vma_count = vmas.len();
> - let mut grouped_vmas: Vec<_> = vmas - .into_iter() -
> .into_group_map_by(|d|d.backup_id.clone()) - .into_values() -
> .collect(); - grouped_vmas.sort_by_key(|d|d[0].backup_id.clone()); +
> let grouped_vmas = vmas.into_iter().fold( + HashMap::new(), + |mut
> grouped: HashMap<String, Vec<VmaBackupArgs>>, vma_args|{ + grouped +
> .entry(vma_args.backup_id.clone()) + .or_default() + .push(vma_args);
> + grouped + }, + ); log::info!( "Found {} backup archive(s) of {}
> different VMID(s):", @@ -311,12 +315,8 @@ fn parse_args() ->
> Result<BackupVmaToPbsArgs, Error> { grouped_vmas.len() ); - for
> vma_group in &grouped_vmas { - log::info!( - "- VMID {}: {} backups",
> - vma_group[0].backup_id, - vma_group.len() - ); + for (vma_group,
> vma_args) in &grouped_vmas { + log::info!("- VMID {}: {} backups",
> vma_group, vma_args.len()); } if !yes {
done
On 04/11/2024 14:06, Fabian Grünbichler wrote:
>> + println!(
>> + "Found {} backup archive(s) of {} different VMID(s):",
>> + total_vma_count,
>> + grouped_vmas.len()
>> + );
> if we don't find any, we should print something else here and exit?
done with `bail!` in v5
On 04/11/2024 14:06, Fabian Grünbichler wrote:
>> + if !yes {
>> + loop {
>> + eprint!("Proceed with the bulk import? (y/n): ");
>> + let mut line = String::new();
>> +
>> + BufReader::new(std::io::stdin()).read_line(&mut line)?;
>> + let trimmed = line.trim();
>> + if trimmed == "y" || trimmed == "Y" {
>> + break;
>> + } else if trimmed == "n" || trimmed == "N" {
>> + bail!("Bulk import was not confirmed.");
>> + }
> this maybe should mimic what we do in proxmox_router when prompting
> for confirmation? e.g., flush stdout, have a default value, ..? should
> we abort after a few loops?
Changed in v5 to mimic the behaviour of the confirmation prompt in
proxmox_router. (bail on invalid input)
Also made Y the default choice.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
next prev parent reply other threads:[~2024-11-11 13:14 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-30 13:55 [pbs-devel] [PATCH vma-to-pbs v4 0/6] " Filip Schauer
2024-10-30 13:55 ` [pbs-devel] [PATCH vma-to-pbs v4 1/6] split BackupVmaToPbsArgs into PbsArgs and VmaBackupArgs Filip Schauer
2024-10-30 13:55 ` [pbs-devel] [PATCH vma-to-pbs v4 2/6] add support for bulk import of a dump directory Filip Schauer
2024-11-04 13:06 ` Fabian Grünbichler
2024-11-11 13:13 ` Filip Schauer [this message]
2024-10-30 13:55 ` [pbs-devel] [PATCH vma-to-pbs v4 3/6] add option to skip vmids whose backups failed to upload Filip Schauer
2024-10-30 13:55 ` [pbs-devel] [PATCH vma-to-pbs v4 4/6] remove hard coded values Filip Schauer
2024-10-30 13:55 ` [pbs-devel] [PATCH vma-to-pbs v4 5/6] use level-based logging instead of println Filip Schauer
2024-10-30 13:55 ` [pbs-devel] [PATCH vma-to-pbs v4 6/6] log device upload progress as a percentage Filip Schauer
2024-11-04 13:09 ` [pbs-devel] partially-applied: [PATCH vma-to-pbs v4 0/6] add support for bulk import of a dump directory Fabian Grünbichler
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=976c12d0-482f-4443-9780-21b02ec50b92@proxmox.com \
--to=f.schauer@proxmox.com \
--cc=pbs-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