[pbs-devel] [PATCH proxmox-backup] api-types: remove influxdb name restrictions

[pbs-devel] [PATCH proxmox-backup] api-types: remove influxdb name restrictions

[Gabriel Goller]

Remove the regex for influxdb organizations and buckets. Influxdb does
not place any constraints on these names and allows all characters. This
allows influxdb organization names with slashes.

Also remove a duplicate comment and add some missing ones.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-api-types/src/metrics.rs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/pbs-api-types/src/metrics.rs b/pbs-api-types/src/metrics.rs
index 6800c23b..23421035 100644
--- a/pbs-api-types/src/metrics.rs
+++ b/pbs-api-types/src/metrics.rs
@@ -12,14 +12,12 @@ pub const METRIC_SERVER_ID_SCHEMA: Schema = StringSchema::new("Metrics Server ID
     .schema();
 
 pub const INFLUXDB_BUCKET_SCHEMA: Schema = StringSchema::new("InfluxDB Bucket.")
-    .format(&PROXMOX_SAFE_ID_FORMAT)
     .min_length(3)
     .max_length(32)
     .default("proxmox")
     .schema();
 
 pub const INFLUXDB_ORGANIZATION_SCHEMA: Schema = StringSchema::new("InfluxDB Organization.")
-    .format(&PROXMOX_SAFE_ID_FORMAT)
     .min_length(3)
     .max_length(32)
     .default("proxmox")
@@ -129,13 +127,14 @@ pub struct InfluxDbHttp {
     pub enable: bool,
     /// The base url of the influxdb server
     pub url: String,
-    /// The Optional Token
     #[serde(skip_serializing_if = "Option::is_none")]
     /// The (optional) API token
     pub token: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    /// Named location where time series data is stored
     pub bucket: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    /// Workspace for a group of users
     pub organization: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     /// The (optional) maximum body size
-- 
2.43.0



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

Re: [pbs-devel] [PATCH proxmox-backup] api-types: remove influxdb name restrictions

[Thomas Lamprecht]

for the subject please do s/name/bucket name/ as otherwise one might think
this is about the ID of an InfluxDB metric server.

Am 19/04/2024 um 15:18 schrieb Gabriel Goller:
> Remove the regex for influxdb organizations and buckets. Influxdb does
> not place any constraints on these names and allows all characters. This
> allows influxdb organization names with slashes.
> 
> Also remove a duplicate comment and add some missing ones.


did you run into this or some user (forum link, bug report)?

What about Proxmox VE, that also has a InfluxDB 


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

Re: [pbs-devel] [PATCH proxmox-backup] api-types: remove influxdb name restrictions

[Gabriel Goller]

On Sun Apr 21, 2024 at 1:28 PM CEST, Thomas Lamprecht wrote:
> for the subject please do s/name/bucket name/ as otherwise one might think
> this is about the ID of an InfluxDB metric server.

Will do.

> Am 19/04/2024 um 15:18 schrieb Gabriel Goller:
> > Remove the regex for influxdb organizations and buckets. Influxdb does
> > not place any constraints on these names and allows all characters. This
> > allows influxdb organization names with slashes.
> > 
> > Also remove a duplicate comment and add some missing ones.
>
>
> did you run into this or some user (forum link, bug report)?

From a forum thread here (will also include this in the next patch): 
https://forum.proxmox.com/threads/influx-db-organization-doesnt-allow-slash.145402/

> What about Proxmox VE, that also has a InfluxDB 

In Proxmox VE there are no restrictions to bucket/organization names.



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

Re: [pbs-devel] [PATCH proxmox-backup] api-types: remove influxdb name restrictions

[Thomas Lamprecht]

Am 22/04/2024 um 09:20 schrieb Gabriel Goller:
> On Sun Apr 21, 2024 at 1:28 PM CEST, Thomas Lamprecht wrote:
>> for the subject please do s/name/bucket name/ as otherwise one might think
>> this is about the ID of an InfluxDB metric server.
> 
> Will do.
> 
>> Am 19/04/2024 um 15:18 schrieb Gabriel Goller:
>>> Remove the regex for influxdb organizations and buckets. Influxdb does
>>> not place any constraints on these names and allows all characters. This
>>> allows influxdb organization names with slashes.
>>>
>>> Also remove a duplicate comment and add some missing ones.
>>
>>
>> did you run into this or some user (forum link, bug report)?
> 
> From a forum thread here (will also include this in the next patch): 
> https://forum.proxmox.com/threads/influx-db-organization-doesnt-allow-slash.145402/
> 
>> What about Proxmox VE, that also has a InfluxDB 
> 
> In Proxmox VE there are no restrictions to bucket/organization names.
> 

OK, it would be nice to mention that here, because having the same schema
and behavior across products is a merit in itself.


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

Re: [pbs-devel] [PATCH proxmox-backup] api-types: remove influxdb name restrictions

[Gabriel Goller]

On Mon Apr 22, 2024 at 9:26 AM CEST, Thomas Lamprecht wrote:
> Am 22/04/2024 um 09:20 schrieb Gabriel Goller:
> > On Sun Apr 21, 2024 at 1:28 PM CEST, Thomas Lamprecht wrote:
> >> for the subject please do s/name/bucket name/ as otherwise one might think
> >> this is about the ID of an InfluxDB metric server.
> > 
> > Will do.
> > 
> >> Am 19/04/2024 um 15:18 schrieb Gabriel Goller:
> >>> Remove the regex for influxdb organizations and buckets. Influxdb does
> >>> not place any constraints on these names and allows all characters. This
> >>> allows influxdb organization names with slashes.
> >>>
> >>> Also remove a duplicate comment and add some missing ones.
> >>
> >>
> >> did you run into this or some user (forum link, bug report)?
> > 
> > From a forum thread here (will also include this in the next patch): 
> > https://forum.proxmox.com/threads/influx-db-organization-doesnt-allow-slash.145402/
> > 
> >> What about Proxmox VE, that also has a InfluxDB 
> > 
> > In Proxmox VE there are no restrictions to bucket/organization names.
> > 
>
> OK, it would be nice to mention that here, because having the same schema
> and behavior across products is a merit in itself.

Thanks for the review!
Submitted a v2.


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