public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Hannes Duerr <h.duerr@proxmox.com>
To: Lou.Lecrivain@wdz.de, lou.lecrivain@orange.fr,
	pve-devel@lists.proxmox.com
Cc: jonatan.crystall@gwdg.de
Subject: Re: [pve-devel] [!!ACHTUNG extern!!] - Re: [PATCH pve-network v3 1/3] ipam: nautobot: base plugin
Date: Wed, 19 Mar 2025 17:01:59 +0100	[thread overview]
Message-ID: <c6ad5335-d12b-4ae0-ad60-e96346684423@proxmox.com> (raw)
In-Reply-To: <FR2PPF45D006270BAE05B48B2B7C1D2E18F85D92@FR2PPF45D006270.DEUP281.PROD.OUTLOOK.COM>


On 3/19/25 10:50, Lou.Lecrivain@wdz.de wrote:
> Re,
>
> Ok so. Answering on that.
>
> By default, the status "Active" already exists for fresh installs. However,
> since the users are free to define whatever status they want (and delete the
> pre-existing ones!), I thought it'd be best if we checked it did exist first.
> There could be a case where we use a Nautobot instance where the status
> "Active" does not exist. But I agree it's a bit far-fetched.
>
> I wrote it this way since IMO it is better to have an explicit error message
> rather than something like "incorrect API request" on subsequent API calls with
> potentially no more details.

Sounds sensible, thanks for the clarification!

 > sub default_headers {
 >     my ($plugin_config) = @_;
 >     my $token = $plugin_config->{token};
 >     return [
 >     'Content-Type' => "application/json", 'Authorization' => "token 
$token",
 >     'Accept' => "application/json"
 >     ];

Another question, why did you set Accept to application/json here, is 
that necessary?

> HTH
>
> ________________________________________
> De : Hannes Dürr <h.duerr@proxmox.com>
> Envoyé : mercredi 19 mars 2025 10:21
> À : lou.lecrivain@orange.fr <lou.lecrivain@orange.fr>; pve-devel@lists.proxmox.com <pve-devel@lists.proxmox.com>
> Cc : jonatan.crystall@gwdg.de <jonatan.crystall@gwdg.de>; Lecrivain, Lou (WDZ) <Lou.Lecrivain@wdz.de>
> Objet : [!!ACHTUNG extern!!] - Re: [PATCH pve-network v3 1/3] ipam: nautobot: base plugin
>   
>
> On 3/6/25 12:08, lou.lecrivain@orange.fr wrote:
>
> [...]
>
>> +sub verify_api {
>> +    my ($class, $plugin_config) = @_;
>> +
>> +    my $url = $plugin_config->{url};
>> +    my $namespace = $plugin_config->{namespace};
>> +    my $headers = default_headers($plugin_config);
>> +
>> +    # check that the namespace exists AND that default IP active status
>> +    # exists AND that we have indeed API access
>> +    eval {
>> +     get_namespace_id($url, $namespace, $headers, 0)
>> +         // die "namespace $namespace does not exist";
>> +     get_status_id($url, default_ip_status(), $headers, 0)
>> +         // die "default IP status ". default_ip_status() . " not found";
>> +    };
>> +    if ($@) {
>> +     die "Can't use nautobot api: $@";
>> +    }
>> +}
>> +
> [...]
>
>> +sub get_status_id {
>> +    my ($url, $status, $headers, $noerr) = @_;
>> +
>> +    my $result = eval {
>> +     return PVE::Network::SDN::api_request(
>> +         "GET", "$url/extras/statuses/?q=$status", $headers);
>> +    };
> Unfortunately, I don't quite understand what this check actually does,
> or why we need this check.
> The documentation [0] says:
> `Nautobot provides the ability for custom statuses to be defined
> within an organisation to be used on various objects to facilitate
> business workflows around object statuses.`
>
> And as I understand the API call, it checks whether the
> status object ‘Active’ exists and gets its ID. But I don't
> understand why this should be checked with `verify_api`?
> In my opinion, we don't really need this check, unless I'm missing
> something here.
>
> [0]
> https://docs.nautobot.com/projects/core/en/stable/user-guide/platform-functionality/status/
>
>> +    if ($@) {
>> +     die "error while querying for status $status id: $@" if !$noerr;
>> +    }
>> +
>> +    my $data = @{$result->{results}}[0];
>> +    my $internalid = $data->{id};
>> +    return $internalid;
>> +}


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

  parent reply	other threads:[~2025-03-19 16:02 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250306110831.6426-1-lou.lecrivain@orange.fr>
2025-03-06 11:08 ` [pve-devel] " lou.lecrivain--- via pve-devel
2025-03-06 11:08 ` [pve-devel] [PATCH pve-network v3 2/3] ipam: nautobot: add testing for nautobot plugin lou.lecrivain--- via pve-devel
2025-03-06 11:08 ` [pve-devel] [PATCH pve-network v3 3/3] ipam: nautobot: add checks for prefix deletion lou.lecrivain--- via pve-devel
2025-03-06 11:10 ` [pve-devel] [PATCH] pve-docs: add documentation for Nautobot IPAM plugin lou.lecrivain--- via pve-devel
2025-03-06 11:10 ` [pve-devel] [PATCH] pve-manager: add UI dialogs for Nautobot IPAM backend lou.lecrivain--- via pve-devel
2025-03-06 12:13 ` [pve-devel] [PATCH-SERIES pve-network v3] Add Nautobot IPAM support Hannes Dürr
2025-03-06 13:11   ` Lou Lecrivain via pve-devel
     [not found]   ` <FR3PPF52E80A3568CB02FD718D9A4A750B885CA2@FR3PPF52E80A356.DEUP281.PROD.OUTLOOK.COM>
2025-03-06 13:14     ` Hannes Dürr
2025-03-06 12:18 ` Hannes Dürr
2025-03-19  9:21 ` Hannes Dürr
2025-03-19 10:04   ` Lou Lecrivain via pve-devel
     [not found] ` <20250306110831.6426-2-lou.lecrivain@orange.fr>
2025-03-19  9:21   ` [pve-devel] [PATCH pve-network v3 1/3] ipam: nautobot: base plugin Hannes Dürr
2025-03-19  9:50     ` [pve-devel] [!!ACHTUNG extern!!] - " Lou Lecrivain via pve-devel
     [not found]     ` <FR2PPF45D006270BAE05B48B2B7C1D2E18F85D92@FR2PPF45D006270.DEUP281.PROD.OUTLOOK.COM>
2025-03-19 16:01       ` Hannes Duerr [this message]
2025-03-20  9:22         ` [pve-devel] [!!ACHTUNG extern!!] - " Lou Lecrivain via pve-devel

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=c6ad5335-d12b-4ae0-ad60-e96346684423@proxmox.com \
    --to=h.duerr@proxmox.com \
    --cc=Lou.Lecrivain@wdz.de \
    --cc=jonatan.crystall@gwdg.de \
    --cc=lou.lecrivain@orange.fr \
    --cc=pve-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