On 11/4/19 3:51 PM, Stefan Reiter wrote:
> Previous behaviour was bugged and displayed "Node is offline" for all
> non-selected nodes (only 1 can be selected at a time).
> 
> Also fix progress window to show correct number of nodes in backup job.
> 
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
> 
> I cannot remember my past reasoning for that conditional, but I don't see how
> that ever made sense...
> 

applied, then refactored/rewrote that part slightly as it was a bit to
strange to me, especially the total job count reduction during job
execution. I know do as much checks in front, and made the jobNode &
all node cases doing the same thing.


What could be still done is to do something like (short'n ugly written)

await Promise.all(nodes.map(node => new Promise((resolve, reject) =>
    Proxmox.Utils.API2Request({
        ...
    })
));

Ext.Msg.hide(); 
if (errors.length > 0) {
    ...
}

To move out the if (jobsStarted == jobTotalCount) {
check from the API request callback, would give it a bit better flow
and nicer to read (if written out and not just shortn'ugly like my
example above ^^

>  www/manager6/dc/Backup.js | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
> index f61ff39c..0854a07e 100644
> --- a/www/manager6/dc/Backup.js
> +++ b/www/manager6/dc/Backup.js
> @@ -432,22 +432,23 @@ Ext.define('PVE.dc.BackupView', {
>           job.all = job.all === true ? 1 : 0;
>  
>           var errors = [];
> -         var inProgress = allNodes.length;
> +         var jobCount = jobNode === undefined ? allNodes.length : 1;
> +         var inProgress = jobCount;
>  
>           Ext.Msg.show({
>               title: gettext('Please wait...'),
>               closable: false,
>               progress: true
>           });
> -         Ext.Msg.updateProgress(0, '0/' + allNodes.length);
> +         Ext.Msg.updateProgress(0, '0/' + jobCount);
>  
>           var postRequest = function () {
>               inProgress++;
>  
> -             Ext.Msg.updateProgress(inProgress/allNodes.length,
> -                 inProgress + '/' + allNodes.length);
> +             Ext.Msg.updateProgress(inProgress/jobCount,
> +                 inProgress + '/' + jobCount);
>  
> -             if (inProgress == allNodes.length) {
> +             if (inProgress == jobCount) {
>                   Ext.Msg.hide();
>                   if (errors !== undefined && errors.length > 0) {
>                       Ext.Msg.alert('Error', 'Some errors have been 
> encountered:<br />---<br />'
> @@ -457,8 +458,11 @@ Ext.define('PVE.dc.BackupView', {
>           }
>  
>           allNodes.forEach(node => {
> -             if (node.status !== 'online' ||
> -                 (jobNode !== undefined && jobNode !== node.node)) {
> +             if (jobNode !== undefined && jobNode !== node.node) {
> +                 return;
> +             }
> +
> +             if (node.status !== 'online') {
>                   errors.push(node.node + ": " + gettext("Node is offline"));
>                   return;
>               }
> 


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

Reply via email to