[pbs-devel] [PATCH proxmox-backup v3 07/23] api/datastore: move s3 index upload helper to datastore backend

[Christian Ebner]

On 11/6/25 10:38 AM, Fabian Grünbichler wrote:
> On November 5, 2025 1:22 pm, Christian Ebner wrote:
>> In an effort to decouple the api implementation from the backend
>> implementation and deduplicate code.
>>
>> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
>> ---
>> changes since version 2:
>> - no changes
>>
>>   pbs-datastore/src/datastore.rs | 26 ++++++++++++++++++++++++++
>>   src/api2/backup/environment.rs | 32 ++++++++++----------------------
>>   src/server/pull.rs             | 14 ++------------
>>   3 files changed, 38 insertions(+), 34 deletions(-)
>>
>> diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
>> index 0d738f0ac..343f49f36 100644
>> --- a/pbs-datastore/src/datastore.rs
>> +++ b/pbs-datastore/src/datastore.rs
>> @@ -223,6 +223,32 @@ pub enum DatastoreBackend {
>>       S3(Arc<S3Client>),
>>   }
>>   
>> +impl DatastoreBackend {
>> +    /// Reads the index file and uploads it to the S3 backend.
>> +    ///
>> +    /// Returns with error if the backend variant is not S3.
>> +    pub async fn s3_upload_index(&self, backup_dir: &BackupDir, name: &str) -> Result<(), Error> {
> 
> the interface here would be nicer/more ergonomic if this would just be
> 
> datastore.upload_index_to_backend(backend, backup_dir, name) -> Result<bool, Error>
> 
> and implemented as NOP for filesystem backends.. but I suspect we'd need
> to change this once more if we integrate the backend more directly into
> the datastore in the next series?

Yes, that is what it was initially, although I then opted for this to 
return the error and named it to be s3 specific for it to be clear that 
this should only be used for that case. But you are correct, once the 
backend caching is tied to the datastore, this must be adapted anyways.

So if fine by you, I would opt to keep as is for this patch series.