Re: [pve-devel] [PATCH v1 pve-storage 0/8] Base Module + Documentation for PVE::Storage::Plugin API

[Wolfgang Bumiller <w.bumiller@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