[pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: ui: sync job: edit rate limit based on sync direction

[pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: ui: sync job: edit rate limit based on sync direction

[Christian Ebner]

Commit 9aa213b8 ("ui: sync job: adapt edit window to be used for pull
and push") adapted the sync job edit so jobs in both, push and pull
can be edited using the same window. This however did not include the
switching of the direction to which the http client rate limit is
applied to.

Fix this by further adding the edit field for `rate-out` and
conditionally hide the less useful rate limit direction (rate-out for
pull and rate-in for push). This allows to preserve the values if
explicitly set via the sync job config.

Reported in the community forum:
https://forum.proxmox.com/threads/163414/

Fixes: 9aa213b8 ("ui: sync job: adapt edit window to be used for pull and push")
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1 (thanks at Dominik for feedback):
- add additional rate limit field, but hide less useful one based on
  sync direction
- preserve values of unchanged rate limit for less useful direction if
  set via config.

 www/window/SyncJobEdit.js | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index bcd2f2fb2..683babaf2 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -72,9 +72,16 @@ Ext.define('PBS.window.SyncJobEdit', {
 
 	init: function() {
 	    let view = this.getView();
-	    if (view.syncDirectionPush && view.datastore !== undefined) {
-		let localNs = view.down('pbsNamespaceSelector[name=ns]').getValue();
-		view.down('pbsGroupFilter').setLocalNamespace(view.datastore, localNs);
+	    // Cannot use cbind to hide rate limit field depending on sync direction,
+	    // set it in init as workaround.
+	    if (view.syncDirectionPush) {
+		view.down('pmxBandwidthField[name=rate-in]').setHidden(true);
+		if (view.datastore !== undefined) {
+		    let localNs = view.down('pbsNamespaceSelector[name=ns]').getValue();
+		    view.down('pbsGroupFilter').setLocalNamespace(view.datastore, localNs);
+		}
+	    } else {
+		view.down('pmxBandwidthField[name=rate-out]').setHidden(true);
 	    }
 	},
     },
@@ -89,6 +96,15 @@ Ext.define('PBS.window.SyncJobEdit', {
 	me.callParent([values]);
     },
 
+    getVisibleRateLimitField: function() {
+	let me = this;
+	if (me.syncDirectionPush) {
+	    return me.down('field[name=rate-out]');
+	} else {
+	    return me.down('field[name=rate-in]');
+	}
+    },
+
     items: {
 	xtype: 'tabpanel',
 	bodyPadding: 10,
@@ -105,6 +121,7 @@ Ext.define('PBS.window.SyncJobEdit', {
 		    }
 		    if (!me.isCreate) {
 			PBS.Utils.delete_if_default(values, 'rate-in');
+			PBS.Utils.delete_if_default(values, 'rate-out');
 			PBS.Utils.delete_if_default(values, 'remote');
 			if (typeof values.delete === 'string') {
 			    values.delete = values.delete.split(',');
@@ -189,7 +206,17 @@ Ext.define('PBS.window.SyncJobEdit', {
 			fieldLabel: gettext('Rate Limit'),
 			emptyText: gettext('Unlimited'),
 			submitAutoScaledSizeUnit: true,
-			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too
+			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
+			// further hide rate limit field depending on sync direction in controller init.
+		    },
+		    {
+			xtype: 'pmxBandwidthField',
+			name: 'rate-out',
+			fieldLabel: gettext('Rate Limit'),
+			emptyText: gettext('Unlimited'),
+			submitAutoScaledSizeUnit: true,
+			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
+			// further hide rate limit field depending on sync direction in controller init.
 		    },
 		],
 
@@ -221,7 +248,7 @@ Ext.define('PBS.window.SyncJobEdit', {
 				let me = this;
 				let form = me.up('pbsSyncJobEdit');
 				let nsField = form.down('field[name=remote-ns]');
-				let rateLimitField = form.down('field[name=rate-in]');
+				let rateLimitField = form.getVisibleRateLimitField();
 				let remoteField = form.down('field[name=remote]');
 				let storeField = form.down('field[name=remote-store]');
 
@@ -263,7 +290,7 @@ Ext.define('PBS.window.SyncJobEdit', {
 				let me = this;
 				let remoteStoreField = me.up('pbsSyncJobEdit').down('field[name=remote-store]');
 				remoteStoreField.setRemote(value);
-				let rateLimitField = me.up('pbsSyncJobEdit').down('field[name=rate-in]');
+				let rateLimitField = me.up('pbsSyncJobEdit').getVisibleRateLimitField();
 				rateLimitField.setDisabled(!value);
 				if (!value) {
 				    rateLimitField.setValue(null);
-- 
2.39.5



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

[pbs-devel] [PATCH v2 proxmox-backup 2/2] docs: mention how to set the push sync jobs rate limit

[Christian Ebner]

Explicitly mention how to set the rate limit for sync jobs in push
direction to avoid possible confusion.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 docs/managing-remotes.rst | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/docs/managing-remotes.rst b/docs/managing-remotes.rst
index f8012636e..a784fcc00 100644
--- a/docs/managing-remotes.rst
+++ b/docs/managing-remotes.rst
@@ -227,13 +227,16 @@ Bandwidth Limit
 
 Syncing a datastore to an archive can produce a lot of traffic and impact other
 users of the network. In order to avoid network or storage congestion, you can
-limit the bandwidth of the sync job by setting the ``rate-in`` option either in
-the web interface or using the ``proxmox-backup-manager`` command-line tool:
+limit the bandwidth of a sync job in pull direction by setting the ``rate-in``
+option either in the web interface or using the ``proxmox-backup-manager``
+command-line tool:
 
 .. code-block:: console
 
     # proxmox-backup-manager sync-job update ID --rate-in 20MiB
 
+For sync jobs in push direction use the ``rate-out`` option instead.
+
 Sync Direction Push
 ^^^^^^^^^^^^^^^^^^^
 
-- 
2.39.5



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

Re: [pbs-devel] [PATCH v2 proxmox-backup 1/2] fix: ui: sync job: edit rate limit based on sync direction

[Dominik Csapak]

Looks good to me now, if we want to have a unified parameter here,
we can still add it later too.

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 3/18/25 10:47, Christian Ebner wrote:
> Commit 9aa213b8 ("ui: sync job: adapt edit window to be used for pull
> and push") adapted the sync job edit so jobs in both, push and pull
> can be edited using the same window. This however did not include the
> switching of the direction to which the http client rate limit is
> applied to.
> 
> Fix this by further adding the edit field for `rate-out` and
> conditionally hide the less useful rate limit direction (rate-out for
> pull and rate-in for push). This allows to preserve the values if
> explicitly set via the sync job config.
> 
> Reported in the community forum:
> https://forum.proxmox.com/threads/163414/
> 
> Fixes: 9aa213b8 ("ui: sync job: adapt edit window to be used for pull and push")
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 1 (thanks at Dominik for feedback):
> - add additional rate limit field, but hide less useful one based on
>    sync direction
> - preserve values of unchanged rate limit for less useful direction if
>    set via config.
> 
>   www/window/SyncJobEdit.js | 39 +++++++++++++++++++++++++++++++++------
>   1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index bcd2f2fb2..683babaf2 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -72,9 +72,16 @@ Ext.define('PBS.window.SyncJobEdit', {
>   
>   	init: function() {
>   	    let view = this.getView();
> -	    if (view.syncDirectionPush && view.datastore !== undefined) {
> -		let localNs = view.down('pbsNamespaceSelector[name=ns]').getValue();
> -		view.down('pbsGroupFilter').setLocalNamespace(view.datastore, localNs);
> +	    // Cannot use cbind to hide rate limit field depending on sync direction,
> +	    // set it in init as workaround.
> +	    if (view.syncDirectionPush) {
> +		view.down('pmxBandwidthField[name=rate-in]').setHidden(true);
> +		if (view.datastore !== undefined) {
> +		    let localNs = view.down('pbsNamespaceSelector[name=ns]').getValue();
> +		    view.down('pbsGroupFilter').setLocalNamespace(view.datastore, localNs);
> +		}
> +	    } else {
> +		view.down('pmxBandwidthField[name=rate-out]').setHidden(true);
>   	    }
>   	},
>       },
> @@ -89,6 +96,15 @@ Ext.define('PBS.window.SyncJobEdit', {
>   	me.callParent([values]);
>       },
>   
> +    getVisibleRateLimitField: function() {
> +	let me = this;
> +	if (me.syncDirectionPush) {
> +	    return me.down('field[name=rate-out]');
> +	} else {
> +	    return me.down('field[name=rate-in]');
> +	}
> +    },
> +
>       items: {
>   	xtype: 'tabpanel',
>   	bodyPadding: 10,
> @@ -105,6 +121,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   		    }
>   		    if (!me.isCreate) {
>   			PBS.Utils.delete_if_default(values, 'rate-in');
> +			PBS.Utils.delete_if_default(values, 'rate-out');
>   			PBS.Utils.delete_if_default(values, 'remote');
>   			if (typeof values.delete === 'string') {
>   			    values.delete = values.delete.split(',');
> @@ -189,7 +206,17 @@ Ext.define('PBS.window.SyncJobEdit', {
>   			fieldLabel: gettext('Rate Limit'),
>   			emptyText: gettext('Unlimited'),
>   			submitAutoScaledSizeUnit: true,
> -			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too
> +			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
> +			// further hide rate limit field depending on sync direction in controller init.
> +		    },
> +		    {
> +			xtype: 'pmxBandwidthField',
> +			name: 'rate-out',
> +			fieldLabel: gettext('Rate Limit'),
> +			emptyText: gettext('Unlimited'),
> +			submitAutoScaledSizeUnit: true,
> +			// NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
> +			// further hide rate limit field depending on sync direction in controller init.
>   		    },
>   		],
>   
> @@ -221,7 +248,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   				let me = this;
>   				let form = me.up('pbsSyncJobEdit');
>   				let nsField = form.down('field[name=remote-ns]');
> -				let rateLimitField = form.down('field[name=rate-in]');
> +				let rateLimitField = form.getVisibleRateLimitField();
>   				let remoteField = form.down('field[name=remote]');
>   				let storeField = form.down('field[name=remote-store]');
>   
> @@ -263,7 +290,7 @@ Ext.define('PBS.window.SyncJobEdit', {
>   				let me = this;
>   				let remoteStoreField = me.up('pbsSyncJobEdit').down('field[name=remote-store]');
>   				remoteStoreField.setRemote(value);
> -				let rateLimitField = me.up('pbsSyncJobEdit').down('field[name=rate-in]');
> +				let rateLimitField = me.up('pbsSyncJobEdit').getVisibleRateLimitField();
>   				rateLimitField.setDisabled(!value);
>   				if (!value) {
>   				    rateLimitField.setValue(null);



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