From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
	by lore.proxmox.com (Postfix) with ESMTPS id 838B81FF164
	for <inbox@lore.proxmox.com>; Fri, 11 Apr 2025 15:27:48 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 9992D1AF88;
	Fri, 11 Apr 2025 15:27:41 +0200 (CEST)
Date: Fri, 11 Apr 2025 15:27:38 +0200
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Max Carrara <m.carrara@proxmox.com>
Message-ID: <tpzhyuwoofx5w5euthtvqby7jvsifbshwwqm7hyxcwdzto5vm4@oamylugkplg7>
References: <20250326142059.261938-1-m.carrara@proxmox.com>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20250326142059.261938-1-m.carrara@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.081 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 0.001 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.001 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.001 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
Subject: Re: [pve-devel] [PATCH v1 pve-storage 0/8] Base Module +
 Documentation for PVE::Storage::Plugin API
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Cc: pve-devel@lists.proxmox.com
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>

On Wed, Mar 26, 2025 at 03:20:51PM +0100, Max Carrara wrote:
> Abstract Base Module + Documentation for PVE::Storage::Plugin API - v1
> ======================================================================
> 
> This series adds a base module (interface) for PVE::Storage::Plugin,
> named PVE::Storage::PluginBase, which lists all methods that are part of
> the Storage Plugin API. Additionally, docstrings for every method are
> added in order to provide context for (third-party) developers, as
> sometimes a complete roundtrip through our code is otherwise necessary
> in order to (really) understand what a method does.
> 
> Special thanks go to @Maximilano, who has helped me out a lot with this
> series!
> 
> Essentially, the base module is similar to PVE::LXC::Plugin [1], with
> the exception that docstrings are added.
> 
> Instead of documenting every single parameter separately, common /
> recurring method parameters are listed and explained at the very top of
> the file. The caching mechanism of a few methods is also described in
> the top-level documentation.
> 
> Any feedback regarding the docs is highly appreciated -- I hope that we
> haven't overlooked anything. We tried to keep things concise unless it's
> absolutely necessary to add additional context.
> 
> Also, most (but not all) of the methods `croak` right now if they're
> unimplemented, but I feel like default return values for certain methods
> could be returned instead. Then again, ::Plugin already provides those
> defaults, so I'm not sure if it matters either way.

As a high-level comment: In an API with external plugins, I'd prefer to
avoid default values. They may not apply to all storages. While
technically those would now be documented, future additions to the API
which get inherited with default values are still a problem.


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