public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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

  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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal