[pbs-devel] [PATCH v3 proxmox-backup 2/5] api types: introduce `BackupArchiveName` type

[Fabian Grünbichler]

On November 4, 2024 12:56 pm, Christian Ebner wrote:
> On 10/25/24 14:15, Fabian Grünbichler wrote:
>> On October 24, 2024 10:01 am, Christian Ebner wrote:
>>> Introduces a dedicated wrapper type to be used for backup archive
>>> names instead of plain strings and associated helper methods for
>>> archive type checks and archive name mappings.
>>>
>>> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
 
>> 
>>> +        } else {
>>> +            Ok((format!("{archive_name}.blob"), ArchiveType::Blob))
>> 
>> this last catchall here might be a bit dangerous? it basically makes the
>> introduction of a new archive type collide with any existing blobs that
>> happen to have a file name that ends with that new archive type..
> 
> This is true, but we already have that exact same mapping currently in 
> use (see patch 4, which drops the pre-existing helper). But that was 
> arguably more limited in scope.
> 
> So maybe we might keep the pre-existing helper here instead of 
> implementing `parse_archive_type` for the API type directly?

yeah, that old helper was only used in a single place - the
`proxmox-backup-client restore ..` command. so I guess we need to
special case this there if we want to avoid extending that catchall
fallback to everywhere?