From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter
Date: Fri, 20 Nov 2020 14:32:24 +0100 [thread overview]
Message-ID: <247e7342-8f89-adcf-740b-9ba1767c2822@proxmox.com> (raw)
In-Reply-To: <20201120095049.15194-5-d.csapak@proxmox.com>
On 20.11.20 10:50, Dominik Csapak wrote:
> adds ui support for configuring the External Metric servers.
> for now, influxdb and graphite edit windows do not share code,
> it would be ideal to do that once we add more plugins
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> www/manager6/Makefile | 1 +
> www/manager6/dc/Config.js | 6 +
> www/manager6/dc/MetricServerView.js | 397 ++++++++++++++++++++++++++++
> 3 files changed, 404 insertions(+)
> create mode 100644 www/manager6/dc/MetricServerView.js
>
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 4fa8e1a3..a2514464 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -146,6 +146,7 @@ JSSRC= \
> dc/TokenView.js \
> dc/UserEdit.js \
> dc/UserView.js \
> + dc/MetricServerView.js \
> lxc/CmdMenu.js \
> lxc/Config.js \
> lxc/CreateWizard.js \
> diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
> index 905c3dc0..2fdba743 100644
> --- a/www/manager6/dc/Config.js
> +++ b/www/manager6/dc/Config.js
> @@ -231,6 +231,12 @@ Ext.define('PVE.dc.Config', {
> list_refs_url: '/cluster/firewall/refs',
> itemId: 'firewall-ipset'
> },
> + {
> + xtype: 'pveMetricServerView',
> + title: gettext('Metric Server'),
> + iconCls: 'fa fa-bar-chart',
> + itemId: 'metricservers',
> + },
> {
> xtype: 'pveDcSupport',
> title: gettext('Support'),
> diff --git a/www/manager6/dc/MetricServerView.js b/www/manager6/dc/MetricServerView.js
> new file mode 100644
> index 00000000..894cd18a
> --- /dev/null
> +++ b/www/manager6/dc/MetricServerView.js
> @@ -0,0 +1,397 @@
> +Ext.define('PVE.dc.MetricServerView', {
> + extend: 'Ext.grid.Panel',
> + alias: ['widget.pveMetricServerView'],
> +
> + stateful: true,
> + stateId: 'grid-metricserver',
> +
> + controller: {
> + xclass: 'Ext.app.ViewController',
> +
> + render_type: function(value) {
> + switch(value) {
> + case 'influxdb': return "InfluxDB";
> + case 'graphite': return "Graphite";
> + default: break;
> + }
> + return Proxmox.Utils.unknownText;
nit, that catch-all return could be in the switch default arm - so all function
exits are there - it's a short function and no hard feeling, but stood out to me.
> + },
> +
> + addInfluxDB: function() {
> + let me = this;
> + Ext.create(`PVE.dc.InfluxDBEdit`, {
> + url: `/api2/extjs/cluster/metricserver`,
> + listeners: {
> + destroy: function() {
> + me.reload();
> + }
could be a single line arrow function, for such short definitions they are still visible
enough, IMO.
destroy: () => me.reload(),
> + }
> + }).show();
you could use autoShow: true instead
> + },
> +
> + addGraphite: function() {
> + let me = this;
> + Ext.create(`PVE.dc.GraphiteEdit`, {
> + url: `/api2/extjs/cluster/metricserver`,
> + listeners: {
> + destroy: function() {
> + me.reload();
> + }
same with reload as arrow function
> + }
> + }).show();
same as above (autoShow)
> + },
> +
> + editServer: function() {
> + let me = this;
> + let view = me.getView();
> + let selection = view.getSelection();
> + if (!selection || selection.length < 1) { return; }
please format above if nicely, return and closing } on it's one line.
> +
> + let rec = selection[0];
> +
> + let xtype = me.render_type(rec.data.type);
> + Ext.create(`PVE.dc.${xtype}Edit`, {
> + url: `/api2/extjs/cluster/metricserver/${rec.data.id}`,
> + serverid: rec.data.id,
> + listeners: {
> + destroy: function() {
> + me.reload();
same with reload as arrow function
> + }
> + }
> + }).show();
could be autoShow
> + },
> +
> + reload: function() { this.getView().getStore().load(); },
please format in new lines
> + },
> +
> + store: {
> + autoLoad: true,
> + id: 'metricservers',
> + proxy: {
> + type: 'proxmox',
> + url: '/api2/json/cluster/metricserver',
> + },
> + },
> +
> + columns: [
> + {
> + text: gettext('Name'),
> + flex: 1,
> + dataIndex: 'id',
> + },
> + {
> + text: gettext('Type'),
> + flex: 1,
> + dataIndex: 'type',
> + renderer: 'render_type',
> + },
> + {
> + text: gettext('Enabled'),
> + dataIndex: 'disable',
> + width: 100,
> + renderer: Proxmox.Utils.format_neg_boolean,
> + },
> + {
> + text: gettext('Server'),
> + flex: 1,
> + dataIndex: 'server',
> + },
> + {
> + text: gettext('Port'),
> + flex: 1,
avoid flex for things we know there are fixed boundaries, while I am a fan of
defining flex having all flexed is most of the time also "bad", as in, increases
space between information needlessly so that there's much eye movement required
for getting it on bigger screens.
I discussed this with Dietmar a bit ago for some PBS job grids, and there he was
right and making fixed length columns exactly that, fixed :)
Sometimes a combination of flex and maxWidth can have really good results too.
> + dataIndex: 'port',
> + },
> + ],
> +
> + tbar: [
> + {
> + text: gettext('Add'),
> + menu: [
> + {
> + text: 'Graphite',
> + handler: 'addGraphite',
> + },
> + {
> + text: 'InfluxDB',
> + handler: 'addInfluxDB',
> + },
> + ]
> + },
> + {
> + text: gettext('Edit'),
> + xtype: 'proxmoxButton',
> + handler: 'editServer',
> + disabled: true,
> + },
> + {
> + xtype: 'proxmoxStdRemoveButton',
> + baseurl: `/api2/extjs/cluster/metricserver`,
> + callback: 'reload',
> + }
> + ],
> +
> + listeners: {
> + itemdblclick: 'editServer',
> + },
> +
> + initComponent : function() {
> + var me = this;
> +
> + me.callParent();
> +
> + Proxmox.Utils.monStoreErrors(me, me.getStore());
> + }
> +});
> +
> +Ext.define('PVE.dc.InfluxDBEdit', {
> + extend: 'Proxmox.window.Edit',
> + mixins: ['Proxmox.Mixin.CBind'],
> +
> + subject: 'InfluxDB',
> +
> + cbindData: function() {
> + let me = this;
> + me.isCreate = !me.serverid;
> + me.serverid = me.serverid || "";
> + me.method = me.isCreate ? 'POST' : 'PUT';
> + if (!me.isCreate) {
> + me.subject = `InfluxDB: ${me.serverid}`;
> + }
> + return {};
> + },
> +
> + items: [
> + {
> + xtype: 'inputpanel',
> +
> + onGetValues: function(values) {
> + values.disable = values.enable ? 0 : 1;
> + delete values.enable;
> + return values;
> + },
> +
> + column1: [
> + {
> + xtype: 'hidden',
> + name: 'type',
> + value: 'influxdb',
> + cbind: {
> + submitValue: '{isCreate}',
> + },
> + },
> + {
> + fieldLabel: gettext('Name'),
> + xtype: 'pmxDisplayEditField',
> + name: 'id',
> + allowBlank: false,
> + cbind: {
> + editable: '{isCreate}',
> + value: '{serverid}',
> + },
> + },
> + {
> + fieldLabel: gettext('Enabled'),
> + xtype: 'checkbox',
> + inputValue: 1,
> + uncheckedValue: 0,
> + checked: true,
> + name: 'enable',
> + },
> + ],
> +
> + column2: [
> + {
> + fieldLabel: gettext('Server'),
> + xtype: 'proxmoxtextfield',
> + name: 'server',
> + allowBlank: false,
> + },
> + {
> + fieldLabel: gettext('Port'),
> + xtype: 'proxmoxintegerfield',
> + minValue: 1,
> + maximum: 65536,
> + name: 'port',
> + allowBlank: false,
> + },
> + ],
> +
> + advancedColumn1: [], // has to exists to render any advanced columns
> +
> + advancedColumn2: [
> + {
> + fieldLabel: 'MTU',
> + xtype: 'proxmoxintegerfield',
> + name: 'mtu',
> + minValue: 1,
> + emptyText: '1500',
> + submitEmpty: false,
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + },
> + ]
> + },
> + ],
> +
> + initComponent: function() {
> + let me = this;
> + me.callParent();
> + if (!me.serverid) { return; }
> +
> + me.load({
> + success: function(response, options) {
> + let values = response.result.data;
> + values.enable = !values.disable;
> + me.down('inputpanel').setValues(values);
> + }
> + })
> + }
> +});
> +
> +Ext.define('PVE.dc.GraphiteEdit', {
> + extend: 'Proxmox.window.Edit',
> + mixins: ['Proxmox.Mixin.CBind'],
> +
> + subject: 'Graphite',
> +
> + cbindData: function() {
> + let me = this;
> + me.isCreate = !me.serverid;
> + me.serverid = me.serverid || "";
> + me.method = me.isCreate ? 'POST' : 'PUT';
> + if (!me.isCreate) {
> + me.subject = `Graphite: ${me.serverid}`;
> + }
> + return {};
> + },
> +
> + items: [
> + {
> + xtype: 'inputpanel',
> +
> + onGetValues: function(values) {
> + values.disable = values.enable ? 0 : 1;
> + delete values.enable;
> + return values;
> + },
> +
> + column1: [
> + {
> + xtype: 'hidden',
> + name: 'type',
> + value: 'graphite',
> + cbind: {
> + submitValue: '{isCreate}',
> + },
> + },
> + {
> + fieldLabel: gettext('Name'),
> + xtype: 'pmxDisplayEditField',
> + name: 'id',
> + allowBlank: false,
> + cbind: {
> + editable: '{isCreate}',
> + value: '{serverid}',
> + },
> + },
> + {
> + fieldLabel: gettext('Enabled'),
> + xtype: 'checkbox',
> + inputValue: 1,
> + uncheckedValue: 0,
> + checked: true,
> + name: 'enable',
> + },
> + ],
> +
> + column2: [
> + {
> + fieldLabel: gettext('Server'),
> + xtype: 'proxmoxtextfield',
> + name: 'server',
> + allowBlank: false,
> + },
> + {
> + fieldLabel: gettext('Port'),
> + xtype: 'proxmoxintegerfield',
> + minimum: 1,
> + maximum: 65536,
> + name: 'port',
> + allowBlank: false,
> + },
> + {
> + fieldLabel: gettext('Path'),
> + xtype: 'proxmoxtextfield',
> + emptyText: 'proxmox',
> + name: 'path',
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + }
> + ],
> +
> + advancedColumn1: [
> + {
> + fieldLabel: gettext('Protocol'),
> + xtype: 'proxmoxKVComboBox',
> + name: 'proto',
> + value: '__default__',
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + comboItems: [
> + ['__default__', 'UDP'],
> + ['tcp', 'TCP'],
> + ],
> + listeners: {
> + change: function(field, value) {
> + let me = this;
> + me.up('inputpanel').down('field[name=timeout]').setDisabled(value !== 'tcp');
> + me.up('inputpanel').down('field[name=mtu]').setDisabled(value === 'tcp');
> + },
> + },
> + },
> + ],
> +
> + advancedColumn2: [
> + {
> + fieldLabel: 'MTU',
> + xtype: 'proxmoxintegerfield',
> + name: 'mtu',
> + minimum: 1,
> + emptyText: '1500',
> + submitEmpty: false,
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + },
> + {
> + fieldLabel: gettext('TCP Timeout'),
> + xtype: 'proxmoxintegerfield',
> + name: 'timeout',
> + disabled: true,
> + cbind: {
> + deleteEmpty: '{!isCreate}',
> + },
> + minValue: 1,
> + emptyText: 1,
> + },
> + ],
> + },
> + ],
> +
> + initComponent: function() {
> + let me = this;
> + me.callParent();
> + if (!me.serverid) { return; }
> +
> + me.load({
> + success: function(response, options) {
> + let values = response.result.data;
> + values.enable = !values.disable;
> + me.down('inputpanel').setValues(values);
> + }
> + })
> + }
> +});
>
prev parent reply other threads:[~2020-11-20 13:32 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-20 9:50 [pve-devel] [PATCH manager 0/4] add UI support for editing metric servers Dominik Csapak
2020-11-20 9:50 ` [pve-devel] [PATCH manager 1/4] Status/Plugin: fix jsonschema for MTU Dominik Csapak
2020-11-20 13:02 ` [pve-devel] applied: " Thomas Lamprecht
2020-11-20 9:50 ` [pve-devel] [PATCH manager 2/4] Status/Plugin: add id to schema Dominik Csapak
2020-11-20 13:02 ` [pve-devel] applied: " Thomas Lamprecht
2020-11-20 9:50 ` [pve-devel] [PATCH manager 3/4] api2/cluster: add 'metricserver' api endpoints Dominik Csapak
2020-11-20 13:21 ` Thomas Lamprecht
2020-11-20 9:50 ` [pve-devel] [PATCH manager 4/4] ui: add MetricServerView to Datacenter Dominik Csapak
2020-11-20 13:32 ` Thomas Lamprecht [this message]
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=247e7342-8f89-adcf-740b-9ba1767c2822@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=d.csapak@proxmox.com \
--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