From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id BA0461FF139 for ; Tue, 24 Feb 2026 13:02:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F3D1176BC; Tue, 24 Feb 2026 13:03:17 +0100 (CET) Message-ID: <7d7af590-7ce1-407f-9b7e-563a1a5c38cd@proxmox.com> Date: Tue, 24 Feb 2026 13:03:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [pbs-devel] [PATCH proxmox 2/3] s3-client: parse and return headers for delete object response To: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= , Proxmox Backup Server development discussion References: <20260127122712.505774-1-c.ebner@proxmox.com> <20260127122712.505774-3-c.ebner@proxmox.com> <1771933070.i0upna84z7.astroid@yuna.none> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <1771933070.i0upna84z7.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1771934576363 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.015 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1.179 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.717 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.236 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: BIVWFRW74ALIRGHK2WHNTBZ25NXIKEGE X-Message-ID-Hash: BIVWFRW74ALIRGHK2WHNTBZ25NXIKEGE X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 >> --- >> 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 { >> 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 { >> 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!