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:
[email protected]