You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/11/29 21:03:31 UTC
[GitHub] [guacamole-client] jmuehlner opened a new pull request, #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
jmuehlner opened a new pull request, #779:
URL: https://github.com/apache/guacamole-client/pull/779
As discussed in [GUACAMOLE-1772](https://issues.apache.org/jira/browse/GUACAMOLE-1722) this allows user configuration of the KSM API call timeout.
While I was in there, I also cleaned up some warnings about unchecked null references.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #779:
URL: https://github.com/apache/guacamole-client/pull/779#discussion_r1035467583
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java:
##########
@@ -176,6 +188,20 @@ public boolean getMatchUserRecordsByDomain() throws GuacamoleException {
return environment.getProperty(MATCH_USER_DOMAINS, false);
}
+ /**
+ * Return the minimum number of milliseconds between KSM API calls. If not
+ * otherwise configured, this value will be 60 seconds.
+ *
+ * @return
+ * The minimum number of milliseconds between KSM API calls.
+ *
+ * @throws GuacamoleException
+ * If the value specified within guacamole.properties cannot be
+ * parsed or does not exist.
+ */
+ public long getKsmApiTimeout() throws GuacamoleException {
+ return environment.getProperty(KSM_API_CALL_TIMEOUT, 60000L);
Review Comment:
My thinking would be either keep it as-is (5 secs), or lengthen it to 10 or 15 secs (typical for guac), or somehow make things dynamic such that the rate can gradually back off in a TBD-but-sensible fashion.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #779:
URL: https://github.com/apache/guacamole-client/pull/779#discussion_r1035282766
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java:
##########
@@ -176,6 +188,20 @@ public boolean getMatchUserRecordsByDomain() throws GuacamoleException {
return environment.getProperty(MATCH_USER_DOMAINS, false);
}
+ /**
+ * Return the minimum number of milliseconds between KSM API calls. If not
+ * otherwise configured, this value will be 60 seconds.
+ *
+ * @return
+ * The minimum number of milliseconds between KSM API calls.
+ *
+ * @throws GuacamoleException
+ * If the value specified within guacamole.properties cannot be
+ * parsed or does not exist.
+ */
+ public long getKsmApiTimeout() throws GuacamoleException {
+ return environment.getProperty(KSM_API_CALL_TIMEOUT, 60000L);
Review Comment:
The original default timeout was 5 seconds, which means that under heavy load, an entire monthly API call budget could be used up in a bit over 3 hours (the base subscription includes 2500 calls / month).
At 60 seconds, the budget would stretch to nearly 2 days.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #779:
URL: https://github.com/apache/guacamole-client/pull/779#discussion_r1036200790
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java:
##########
@@ -176,6 +188,20 @@ public boolean getMatchUserRecordsByDomain() throws GuacamoleException {
return environment.getProperty(MATCH_USER_DOMAINS, false);
}
+ /**
+ * Return the minimum number of milliseconds between KSM API calls. If not
+ * otherwise configured, this value will be 60 seconds.
+ *
+ * @return
+ * The minimum number of milliseconds between KSM API calls.
+ *
+ * @throws GuacamoleException
+ * If the value specified within guacamole.properties cannot be
+ * parsed or does not exist.
+ */
+ public long getKsmApiTimeout() throws GuacamoleException {
+ return environment.getProperty(KSM_API_CALL_TIMEOUT, 60000L);
Review Comment:
Eh, I'll go to 10. That will at least buy a little bit more time before a monthly allowance will run out without being too slow to refresh.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] jmuehlner commented on a diff in pull request #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #779:
URL: https://github.com/apache/guacamole-client/pull/779#discussion_r1035455705
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java:
##########
@@ -176,6 +188,20 @@ public boolean getMatchUserRecordsByDomain() throws GuacamoleException {
return environment.getProperty(MATCH_USER_DOMAINS, false);
}
+ /**
+ * Return the minimum number of milliseconds between KSM API calls. If not
+ * otherwise configured, this value will be 60 seconds.
+ *
+ * @return
+ * The minimum number of milliseconds between KSM API calls.
+ *
+ * @throws GuacamoleException
+ * If the value specified within guacamole.properties cannot be
+ * parsed or does not exist.
+ */
+ public long getKsmApiTimeout() throws GuacamoleException {
+ return environment.getProperty(KSM_API_CALL_TIMEOUT, 60000L);
Review Comment:
What interval would you suggest?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper merged pull request #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
Posted by GitBox <gi...@apache.org>.
mike-jumper merged PR #779:
URL: https://github.com/apache/guacamole-client/pull/779
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #779: GUACAMOLE-1772: Allow user configuration of KSM API call timeout.
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #779:
URL: https://github.com/apache/guacamole-client/pull/779#discussion_r1035450916
##########
extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/conf/KsmConfigurationService.java:
##########
@@ -176,6 +188,20 @@ public boolean getMatchUserRecordsByDomain() throws GuacamoleException {
return environment.getProperty(MATCH_USER_DOMAINS, false);
}
+ /**
+ * Return the minimum number of milliseconds between KSM API calls. If not
+ * otherwise configured, this value will be 60 seconds.
+ *
+ * @return
+ * The minimum number of milliseconds between KSM API calls.
+ *
+ * @throws GuacamoleException
+ * If the value specified within guacamole.properties cannot be
+ * parsed or does not exist.
+ */
+ public long getKsmApiTimeout() throws GuacamoleException {
+ return environment.getProperty(KSM_API_CALL_TIMEOUT, 60000L);
Review Comment:
I'm not sure about this. Defaulting to 60 seconds would mean people will not see changes made to records until a full minute has elapsed, which could be surprising behavior for an admin troubleshooting a connection.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org