public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: "Proxmox Backup Server development discussion"
	<pbs-devel@lists.proxmox.com>,
	"Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Subject: Re: [pbs-devel] [PATCH v3 proxmox-backup] fix #6185: client/docs: explicitly mention archive name restrictions
Date: Mon, 17 Mar 2025 16:32:44 +0100	[thread overview]
Message-ID: <ffce48b3-9d8a-451d-a874-8b545f1090ac@proxmox.com> (raw)
In-Reply-To: <1742224310.w3aml8s8r9.astroid@yuna.none>

On 3/17/25 16:18, Fabian Grünbichler wrote:
> On February 21, 2025 3:17 pm, Christian Ebner wrote:
>> Mention in the docs and the api parameter description the limitations
>> for archive name labels. They must contain alphanumerics, hyphens and
>> underscores only to match the regex pattern.
>>
>> Further, explicitly mention the extension in the backup source schema
>> specification, to clearly distinguish this from the label and be in
>> line with the description give in the schema for the cli command.
>>
>> By setting this in the api parameter description, it will be included
>> in the man page for proxmox-backup-client.
>>
>> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6185
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 2:
>> - Also explicitly mention the extension part in the backup source
>>    schema description, thanks @Shannon for noticing!
>> - Adapt commit message accordingly
>>
>>   docs/backup-client.rst                 |  1 +
>>   pbs-client/src/backup_specification.rs | 10 ++++++----
>>   proxmox-backup-client/src/main.rs      |  3 ++-
>>   3 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/backup-client.rst b/docs/backup-client.rst
>> index 1c2a98ed8..e11c0142a 100644
>> --- a/docs/backup-client.rst
>> +++ b/docs/backup-client.rst
>> @@ -169,6 +169,7 @@ the client. The format is:
>>   
>>       <archive-name>.<type>:<source-path>
>>   
>> +The ``archive-name`` must contain alphanumerics, hyphens and underscores only.
>>   Common types are ``.pxar`` for file archives and ``.img`` for block
>>   device images. To create a backup of a block device, run the following command:
> 
> here we call it archive-name, type and source-path, but
>>   
>> diff --git a/pbs-client/src/backup_specification.rs b/pbs-client/src/backup_specification.rs
>> index b6fbc555c..fe5c352e4 100644
>> --- a/pbs-client/src/backup_specification.rs
>> +++ b/pbs-client/src/backup_specification.rs
>> @@ -7,10 +7,12 @@ const_regex! {
>>       BACKUPSPEC_REGEX = r"^([a-zA-Z0-9_-]+\.(pxar|img|conf|log)):(.+)$";
>>   }
>>   
>> -pub const BACKUP_SOURCE_SCHEMA: Schema =
>> -    StringSchema::new("Backup source specification ([<label>:<path>]).")
>> -        .format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
>> -        .schema();
>> +pub const BACKUP_SOURCE_SCHEMA: Schema = StringSchema::new(
>> +    "Backup source specification ([<label.ext>:<path>]), the specification \
>> +    'label' must contain alphanumerics, hyphens and underscores only.",
>> +)
> 
> here we call it label, ext and path ? maybe we could unify these?

Okay, this patch has been applied already [0] but I can send a followup 
for adaption according to your suggestions.

> 
>> +.format(&ApiStringFormat::Pattern(&BACKUPSPEC_REGEX))
>> +.schema();
>>   
>>   pub enum BackupSpecificationType {
>>       PXAR,
>> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
>> index 589a097bb..9077f7bdf 100644
>> --- a/proxmox-backup-client/src/main.rs
>> +++ b/proxmox-backup-client/src/main.rs
>> @@ -628,7 +628,8 @@ fn spawn_catalog_upload(
>>          properties: {
>>              backupspec: {
>>                  type: Array,
>> -               description: "List of backup source specifications ([<label.ext>:<path>] ...)",
>> +               description: "List of backup source specifications ([<label.ext>:<path>] ...), the \
>> +                    specifications 'label' must contain alphanumerics, hyphens and underscores only.",
> 
> same here, and the second 'specifications' should be singular like
> above (or be dropped in both instances, since it doesn't really provide
> any extra information over just "the 'label'" or "the 'archive-name'").
> 
> I think I'd also prefer "can only contain" or "must only contain"
> instead of putting the 'only' at the end.

Acked.

> 
>>                  items: {
>>                      schema: BACKUP_SOURCE_SCHEMA,
>>                  }
>> -- 
>> 2.39.5
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 

[0] 
https://git.proxmox.com/?p=proxmox-backup.git;a=commit;h=abad8e25c46f66106ec07bb3381e63fe8c12f1dd


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

      reply	other threads:[~2025-03-17 15:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-21 14:17 Christian Ebner
2025-03-17 15:18 ` Fabian Grünbichler
2025-03-17 15:32   ` Christian Ebner [this message]

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=ffce48b3-9d8a-451d-a874-8b545f1090ac@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=f.gruenbichler@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