From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 43E421FF138 for ; Mon, 20 Jul 2026 14:49:21 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 7C9DE214B6; Mon, 20 Jul 2026 14:49:20 +0200 (CEST) Message-ID: <89e166aa-7cac-4c58-a401-5b9b174b2093@proxmox.com> Date: Mon, 20 Jul 2026 14:48:46 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH common/manager/proxmox-widget-toolkit/storage v2 00/17] GUI Support for Custom Storage Plugins To: "Max R. Carrara" , pve-devel@lists.proxmox.com References: <20260717154943.696411-1-m.carrara@proxmox.com> Content-Language: en-US From: Jakob Klocker In-Reply-To: <20260717154943.696411-1-m.carrara@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784551702171 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.900 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: NLVJX2VDROFDNMRFGILCIMTN7ZN6GOYR X-Message-ID-Hash: NLVJX2VDROFDNMRFGILCIMTN7ZN6GOYR X-MailFrom: j.klocker@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 7/17/26 5:50 PM, Max R. Carrara wrote: > GUI Support for Custom Storage Plugins - v2 > =========================================== > > Notable Changes Since v1 > ------------------------ > > - Fix the 'sensitive' keyword being stringified to `"1"` when enabled, > instead of being an integer. > (Thanks @Jakob!) > > - Fix a non-breaking hyphen sneaking in, most likely caused by me > copying an already existing string from the UI or some other place. > (Thanks @Jakob!) > > - Implement *experimental* support for `patternProperties` in order to > support the custom schema keywords `x-advanced` and `x-hidden`, which > helps to avoid hard-coding which fields are in the "Advanced" section > and which ones are hidden in the UI, respectively. > > Please note that the commits implementing this are explicitly marked > as RFC and can be dropped if `patternProperties` is undesired. > > I figured that this was a decent use case for `patternProperties`, but > I'm on the fence whether this is something that we actually want. > Feedback is very welcome in that regard. > > Note that the 'sensitive' keyword that's added to the returned plugin > schema is kept as is at the moment -- I'm a bit on the fence here too > whether it should be an 'x-' keyword or whether we should actually > include it in the default schema for JSON schemas overall, as I can > see it being useful in other places in the future too, e.g. the ACME > plugin code (for hiding API keys etc.). > > > Also, given that the two fixes above are rather trivial and no > functionality has been changed, I suggest keeping Jakob's trailer(s) > from v1 in, should this series get applied. > [snip] Thanks for the v2! I've tested the x-hidden and x-advanced properties by adding them to the SSHFS plugin; both work as expected. The sensitive keyword now displays correctly as an integer, and the reported non-breaking hyphen is replaced with a normal dash. I've left comments on the patches. Most are style-nits, but there's one important thing to look into in 2/17. I also noticed that the module-sorting nit I mentioned isn't applied consistently across the codebase, so if it's not relevant here, feel free to ignore it. The text changes and the backend part look good to me. I'm not too familiar with the frontend part of PVE yet, so I can't give much input on that end.