[GitHub] [guacamole-client] mike-jumper commented on a change in pull request #547: GUACAMOLE-1126: Set a maximum of 4 auto-reconnect attempts

[GitBox <gi...@apache.org>]

mike-jumper commented on a change in pull request #547:
URL: https://github.com/apache/guacamole-client/pull/547#discussion_r451290773



##########
File path: guacamole/src/main/webapp/app/client/controllers/clientController.js
##########
@@ -44,6 +44,16 @@ angular.module('client').controller('clientController', ['$scope', '$routeParams
     var requestService         = $injector.get('requestService');
     var tunnelService          = $injector.get('tunnelService');
     var userPageService        = $injector.get('userPageService');
+	
+	/**
+	 * Counter and limiter for maximum auto-reconnect attempts that we should 
+	 * automatically try to establish a connection when CLIENT_AUTO_RECONNECT or 
+	 * TUNNEL_AUTO_RECONNECT error types occur.
+	 *
+	 * @type Number
+	 */
+	var AUTO_RECONNECT_TRY     = 0;
+	var AUTO_RECONNECT_MAXTRY  = 4;

Review comment:
       My main concerns with schema modifications would be ... that they are schema modifications. We certainly did quite a few of those in the past, but doing that now would require a full version number bump and would break our nice release streak of backward compatibility.
   
   Regarding modifications to support arbitrary attributes, that should already pretty much just work. It may be possible to achieve this without serious modifications to the webapp or database schema. If needed, it might make sense to add AngularJS events that extensions could hook into to manipulate this and other behaviors.
   
   I'm not 100% behind the idea behind adding such a limit. Reading through GUACAMOLE-1126, it seems the primary concern motivating this is that each reconnect counts as user activity and therefore allows the session to continue indefinitely. If that is the case, perhaps there is a way that things could be arranged such that reconnects do not count toward session activity.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org