public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Michael Köppl" <m.koeppl@proxmox.com>,
	"Lukas Wagner" <l.wagner@proxmox.com>,
	pdm-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox v5 03/21] api-macro: support #[state] attribute for state injection
Date: Fri, 04 Sep 2026 09:19:16 +0200	[thread overview]
Message-ID: <DL6CL9D01YIT.34QQIWR4MQACX@proxmox.com> (raw)
In-Reply-To: <DL5WOG4L2O78.1VXXWHQA8O29P@proxmox.com>

On Thu Sep 3, 2026 at 8:51 PM CEST, Michael Köppl wrote:
> On Thu Sep 3, 2026 at 1:15 PM CEST, Lukas Wagner wrote:
>
> [snip]
>
>>  fn handle_function_signature(method_info: &mut MethodInfo) -> Result<Ident, Error> {
>> +    let state_params = take_state_attributes(&mut method_info.func.sig);
>> +
>>      let sig = &method_info.func.sig;
>>  
>>      let mut api_method_param = None;
>> @@ -382,7 +474,11 @@ fn handle_function_signature(method_info: &mut MethodInfo) -> Result<Ident, Erro
>>  
>>      let mut param_list = Vec::<(FieldName, ParameterType)>::new();
>>  
>> -    for input in sig.inputs.iter() {
>> +    for (index, input) in sig.inputs.iter().enumerate() {
>> +        if state_params.contains(&index) {
>> +            continue;
>> +        }
>
> Even though it's not something that would likely occur (since this would
> e.g. occur if someone used `#[state] _: PdmApplication` or something
> similar), this would not show any errors from checking the types
> anymore, no? Because the second [loop] expects errors to be handled
> [here], but for state params, this check would simply be skipped. Hope
> I'm not missing something here, but this would probably hide errors.
>

Good thinking!

I tried various patterns, such as 

#[state] _: A
#[state] A(inner): A
#[state] B {inner}: B

and they don't work with the API macro, it seems to trip up somewhere
else - so the issue you described does not seem to matter here.

Now, the next question is, should these patterns work for #[state]
arguments? Probably fine if they don't, but long-term we could think
about supporting these - at least the last two could be handy in some
cases. But we'll probably get by fine without them.

>> +
>>          let (pat_type, pat) = match check_input_type(input) {
>>              Ok(input) => input,
>>              Err(err) => {
>
> [here]
>
>> @@ -410,7 +506,7 @@ fn handle_function_signature(method_info: &mut MethodInfo) -> Result<Ident, Erro
>>          };
>>      }
>>  
>> -    for input in sig.inputs.iter() {
>> +    for (index, input) in sig.inputs.iter().enumerate() {
>>          let (pat_type, pat) = match check_input_type(input) {
>>              Ok(input) => input,
>>              Err(_err) => continue, // we already produced errors above,
>
> [loop]
>
>> @@ -421,17 +517,20 @@ fn handle_function_signature(method_info: &mut MethodInfo) -> Result<Ident, Erro
>>          // values, and our 2 fixed function parameters: `&ApiMethod` and `&mut dyn RpcEnvironment`.
>>          //
>>          // Our strategy is as follows:
>> -        //     1) See if the parameter name also appears in the input schema. In this case we
>> +        //     1) See if the parameter was marked with `#[state]`. Such parameters are filled in
>
> [snip]





  reply	other threads:[~2026-09-04  7:19 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03 11:15 [PATCH datacenter-manager/proxmox v5 00/21] inject application context via API macro for easier integration testing Lukas Wagner
2026-09-03 11:15 ` [PATCH proxmox v5 01/21] router: introduce shared state Lukas Wagner
2026-09-03 11:15 ` [PATCH proxmox v5 02/21] rest-server: allow to inject " Lukas Wagner
2026-09-03 11:15 ` [PATCH proxmox v5 03/21] api-macro: support #[state] attribute for state injection Lukas Wagner
2026-09-03 18:51   ` Michael Köppl
2026-09-04  7:19     ` Lukas Wagner [this message]
2026-09-03 11:15 ` [PATCH proxmox v5 04/21] product-config: add ProductConfig type Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 05/21] context: promote context to a dir-style module Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 06/21] pdm-config: remotes: rename trait methods to read/write/lock Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 07/21] pdm-config: subscriptions: " Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 08/21] remote iterator: pass remote config reader explicitly Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 09/21] context: introduce a ContextFactory to build application context Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 10/21] context: establish PdmApplication object Lukas Wagner
2026-09-03 11:15 ` [PATCH datacenter-manager v5 11/21] context: register PdmApplication in router Lukas Wagner
2026-09-03 18:19   ` Michael Köppl
2026-09-03 11:16 ` [PATCH datacenter-manager v5 12/21] connection: use client factory from PdmApplication handle Lukas Wagner
2026-09-03 18:19   ` Michael Köppl
2026-09-04  8:05     ` Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 13/21] parallel fetcher: pass arguments to closure in a single type Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 14/21] server: migrate existing ParallelFetcher users to use PdmApplication Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 15/21] tests: add helpers for building API-handler-level integration tests Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 16/21] tests: add example tests for SDN API routes Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 17/21] api-cache: add wrapper type Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 18/21] context: provide api-cache on the app object Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 19/21] api: subscriptions: use PdmApplication instead of globals Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 20/21] pdm-config: subscriptions: drop unused accessor functions Lukas Wagner
2026-09-03 18:19   ` Michael Köppl
2026-09-04  8:07     ` Lukas Wagner
2026-09-03 11:16 ` [PATCH datacenter-manager v5 21/21] tests: add example tests for remote subscription management Lukas Wagner
2026-09-03 18:20 ` [PATCH datacenter-manager/proxmox v5 00/21] inject application context via API macro for easier integration testing Michael Köppl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DL6CL9D01YIT.34QQIWR4MQACX@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=m.koeppl@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal