Re: [pbs-devel] [PATCH proxmox v5 2/4] acme: introduce http_status module

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On January 14, 2026 11:29 am, Samuel Rufinatscha wrote:
> On 1/13/26 2:45 PM, Fabian Grünbichler wrote:
>> On January 8, 2026 12:26 pm, Samuel Rufinatscha wrote:
>>> Introduce an internal http_status module with the common ACME HTTP
>>> response codes, and replace use of crate::request::CREATED as well as
>>> direct numeric status code usages.
>> 
>> why not use http::status ? we already have this as dependency pretty
>> much everywhere we do anything HTTP related.. would also for nicer error
>> messages in case the status is not as expected..
>>
> 
> http is only pulled in via the optional client / async-client features,
> not the base impl feature. This code here is gated by impl, where http
> might not be available. Adding http as a hard
> dependency just for the few status code constants feels a bit overkill.
> This matches what we discussed in a previous review round:
> 
> https://lore.proxmox.com/pbs-devel/2b7574fb-a3c5-4119-8fb6-9649881dba15@proxmox.com/ 

your patch makes this crate unusable without enabling either client ;)

http is a small low-level crate for exactly this purpose (a common
implementation of common HTTP related types). we already pull it in
everywhere proxmox-acme is used. in fact, I think it would even make
sense to switch over more things to use http here than just the status
code, but that's a different matter/series..

anyway, I guess we can ignore this for now and discuss switching over to
http at a later point as a series on its own.

> Also, since this is pub(crate) API, I think we can easily switch to
> StatusCode later if http ever becomes a necessary dependency for impl.
> OK with you?
> 
>>>
>>> Signed-off-by: Samuel Rufinatscha <s.rufinatscha@proxmox.com>
>>> ---
>>>   proxmox-acme/src/account.rs      |  8 ++++----
>>>   proxmox-acme/src/async_client.rs |  4 ++--
>>>   proxmox-acme/src/lib.rs          |  2 ++
>>>   proxmox-acme/src/request.rs      | 11 ++++++++++-
>>>   4 files changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/proxmox-acme/src/account.rs b/proxmox-acme/src/account.rs
>>> index d8eb3e73..ea1a3c60 100644
>>> --- a/proxmox-acme/src/account.rs
>>> +++ b/proxmox-acme/src/account.rs
>>> @@ -84,7 +84,7 @@ impl Account {
>>>               method: "POST",
>>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>>               body,
>>> -            expected: crate::request::CREATED,
>>> +            expected: crate::http_status::CREATED,
>>>           };
>>>   
>>>           Ok(NewOrder::new(request))
>>> @@ -106,7 +106,7 @@ impl Account {
>>>               method: "POST",
>>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>>               body,
>>> -            expected: 200,
>>> +            expected: crate::http_status::OK,
>>>           })
>>>       }
>>>   
>>> @@ -131,7 +131,7 @@ impl Account {
>>>               method: "POST",
>>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>>               body,
>>> -            expected: 200,
>>> +            expected: crate::http_status::OK,
>>>           })
>>>       }
>>>   
>>> @@ -321,7 +321,7 @@ impl AccountCreator {
>>>               method: "POST",
>>>               content_type: crate::request::JSON_CONTENT_TYPE,
>>>               body,
>>> -            expected: crate::request::CREATED,
>>> +            expected: crate::http_status::CREATED,
>>>           })
>>>       }
>>>   
>>> diff --git a/proxmox-acme/src/async_client.rs b/proxmox-acme/src/async_client.rs
>>> index 2ff3ba22..043648bb 100644
>>> --- a/proxmox-acme/src/async_client.rs
>>> +++ b/proxmox-acme/src/async_client.rs
>>> @@ -498,7 +498,7 @@ impl AcmeClient {
>>>                   method: "GET",
>>>                   content_type: "",
>>>                   body: String::new(),
>>> -                expected: 200,
>>> +                expected: crate::http_status::OK,
>>>               },
>>>               nonce,
>>>           )
>>> @@ -550,7 +550,7 @@ impl AcmeClient {
>>>                   method: "HEAD",
>>>                   content_type: "",
>>>                   body: String::new(),
>>> -                expected: 200,
>>> +                expected: crate::http_status::OK,
>>>               },
>>>               nonce,
>>>           )
>>> diff --git a/proxmox-acme/src/lib.rs b/proxmox-acme/src/lib.rs
>>> index 6722030c..6051a025 100644
>>> --- a/proxmox-acme/src/lib.rs
>>> +++ b/proxmox-acme/src/lib.rs
>>> @@ -70,6 +70,8 @@ pub use order::Order;
>>>   #[cfg(feature = "impl")]
>>>   pub use order::NewOrder;
>>>   #[cfg(feature = "impl")]
>>> +pub(crate) use request::http_status;
>>> +#[cfg(feature = "impl")]
>>>   pub use request::ErrorResponse;
>>>   
>>>   /// Header name for nonces.
>>> diff --git a/proxmox-acme/src/request.rs b/proxmox-acme/src/request.rs
>>> index dadfc5af..341ce53e 100644
>>> --- a/proxmox-acme/src/request.rs
>>> +++ b/proxmox-acme/src/request.rs
>>> @@ -1,7 +1,6 @@
>>>   use serde::Deserialize;
>>>   
>>>   pub(crate) const JSON_CONTENT_TYPE: &str = "application/jose+json";
>>> -pub(crate) const CREATED: u16 = 201;
>>>   
>>>   /// A request which should be performed on the ACME provider.
>>>   pub(crate) struct Request {
>>> @@ -21,6 +20,16 @@ pub(crate) struct Request {
>>>       pub(crate) expected: u16,
>>>   }
>>>   
>>> +/// Common HTTP status codes used in ACME responses.
>>> +pub(crate) mod http_status {
>>> +    /// 200 OK
>>> +    pub(crate) const OK: u16 = 200;
>>> +    /// 201 Created
>>> +    pub(crate) const CREATED: u16 = 201;
>>> +    /// 204 No Content
>>> +    pub(crate) const NO_CONTENT: u16 = 204;
>>> +}
>>> +
>>>   /// An ACME error response contains a specially formatted type string, and can optionally
>>>   /// contain textual details and a set of sub problems.
>>>   #[derive(Clone, Debug, Deserialize)]
>>> -- 
>>> 2.47.3
>>>
>>>
>>>
>>> _______________________________________________
>>> 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
>> 
>> 
> 
> 
> 


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