Re: [pbs-devel] [PATCH proxmox 2/3] s3-client: parse and return headers for delete object response

[Christian Ebner <c.ebner@proxmox.com>]

On 2/24/26 12:44 PM, Fabian Grünbichler wrote:
> On January 27, 2026 1:27 pm, Christian Ebner wrote:
>> Mimic the response elements from the list objects parsing in the
>> delete object response.
>>
>> In preparation for being able to perform delete objects via
>> individual delete object api calls, if the respective provider quirk
>> is set in the client options.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>>   proxmox-s3-client/src/client.rs          |  9 +++++----
>>   proxmox-s3-client/src/response_reader.rs | 21 +++++++++++++++++++--
>>   2 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs
>> index 83176b39..3d0af5d6 100644
>> --- a/proxmox-s3-client/src/client.rs
>> +++ b/proxmox-s3-client/src/client.rs
>> @@ -29,8 +29,9 @@ use crate::aws_sign_v4::AWS_SIGN_V4_DATETIME_FORMAT;
>>   use crate::aws_sign_v4::{aws_sign_v4_signature, aws_sign_v4_uri_encode};
>>   use crate::object_key::S3ObjectKey;
>>   use crate::response_reader::{
>> -    CopyObjectResponse, DeleteObjectsResponse, GetObjectResponse, HeadObjectResponse,
>> -    ListBucketsResponse, ListObjectsV2Response, PutObjectResponse, ResponseReader,
>> +    CopyObjectResponse, DeleteObjectsResponse, DeletedObject, GetObjectResponse,
>> +    HeadObjectResponse, ListBucketsResponse, ListObjectsV2Response, PutObjectResponse,
>> +    ResponseReader,
>>   };
>>   
>>   /// Default timeout for s3 api requests.
>> @@ -544,7 +545,7 @@ impl S3Client {
>>   
>>       /// Removes an object from a bucket.
>>       /// See reference docs: https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObject.html
>> -    pub async fn delete_object(&self, object_key: S3ObjectKey) -> Result<(), Error> {
>> +    pub async fn delete_object(&self, object_key: S3ObjectKey) -> Result<DeletedObject, Error> {
>>           let object_key = object_key.to_full_key(&self.options.common_prefix);
>>           let request = Request::builder()
>>               .method(Method::DELETE)
>> @@ -553,7 +554,7 @@ impl S3Client {
>>   
>>           let response = self.send(request, None).await?;
>>           let response_reader = ResponseReader::new(response);
>> -        response_reader.delete_object_response().await
>> +        response_reader.delete_object_response(object_key).await
>>       }
>>   
>>       /// Delete multiple objects from a bucket using a single HTTP request.
>> diff --git a/proxmox-s3-client/src/response_reader.rs b/proxmox-s3-client/src/response_reader.rs
>> index 7066c33b..be7c0950 100644
>> --- a/proxmox-s3-client/src/response_reader.rs
>> +++ b/proxmox-s3-client/src/response_reader.rs
>> @@ -361,7 +361,10 @@ impl ResponseReader {
>>       /// Read and parse the delete object response.
>>       ///
>>       /// Returns with error if an unexpected status code is encountered.
>> -    pub(crate) async fn delete_object_response(self) -> Result<(), Error> {
>> +    pub(crate) async fn delete_object_response(
>> +        self,
>> +        key: S3ObjectKey,
>> +    ) -> Result<DeletedObject, Error> {
>>           let (parts, _body) = self.response.into_parts();
>>   
>>           match parts.status {
>> @@ -369,7 +372,21 @@ impl ResponseReader {
>>               status_code => bail!("unexpected status code {status_code}"),
> 
> the changes of this and the following patch itself look fine (albeit
> breaking ;)), but the error handling here seems a bit weird.. especially
> combined with the parts in proxmox-backup?
> 
> could we maybe improve things instead of basically swallowing all
> details and saying "some objects couldn't be deleted"?
> 
> AFAICT `code` in DeleteObjectsResponse is never set as part of this
> series?

Okay, might be hard to encode which objects failed deletion in the error 
message, but that could be logged instead and maybe the number of failed 
object deletes included in the error message, to give some context.

Anyhow, will see to improve this and send a new version, thanks!