You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "ronansalmon (via GitHub)" <gi...@apache.org> on 2023/02/06 16:43:01 UTC

[GitHub] [guacamole-client] ronansalmon opened a new pull request, #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

ronansalmon opened a new pull request, #794:
URL: https://github.com/apache/guacamole-client/pull/794

   Add missing optional configuration options from docker start.sh script


-- 
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] ronansalmon commented on a diff in pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "ronansalmon (via GitHub)" <gi...@apache.org>.
ronansalmon commented on code in PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#discussion_r1108178184


##########
guacamole-docker/bin/start.sh:
##########
@@ -1064,6 +1069,11 @@ fi
 set_property "guacd-hostname" "$GUACD_HOSTNAME"
 set_property "guacd-port"     "$GUACD_PORT"
 
+# A comma-separated list of the identifiers of authentication providers that
+# should be allowed to fail internally without aborting the authentication process
+set_property "skip-if-unavailable"     "$SKIP_IF_UNAVAILABLE"

Review Comment:
   makes sense! 



-- 
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] ronansalmon commented on a diff in pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "ronansalmon (via GitHub)" <gi...@apache.org>.
ronansalmon commented on code in PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#discussion_r1108507402


##########
guacamole-docker/bin/start.sh:
##########
@@ -1164,6 +1174,19 @@ if [ -n "$API_SESSION_TIMEOUT" ]; then
     associate_apisessiontimeout
 fi
 
+# Maximum number of bytes to accept within the entity body of any particular HTTP request
+set_optional_property "api-max-request-size" "$API_MAX_REQUEST_SIZE"
+
+# A comma-separated whitelist of language keys to allow as display language 

Review Comment:
   ok
   



-- 
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] ronansalmon commented on pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "ronansalmon (via GitHub)" <gi...@apache.org>.
ronansalmon commented on PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#issuecomment-1434702228

   ok, squashed the commits !


-- 
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] ronansalmon commented on a diff in pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "ronansalmon (via GitHub)" <gi...@apache.org>.
ronansalmon commented on code in PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#discussion_r1098311107


##########
guacamole-docker/bin/start.sh:
##########
@@ -1164,6 +1174,16 @@ if [ -n "$API_SESSION_TIMEOUT" ]; then
     associate_apisessiontimeout
 fi
 
+# Maximum number of bytes to accept within the entity body of any particular HTTP request
+set_optional_property "api-max-request-size" "$API_MAX_REQUEST_SIZE"
+
+# A comma-separated whitelist of language keys to allow as display language 
+# choices within the Guacamole interface
+set_optional_property "allowed-languages" "$ALLOWED_LANGUAGES"
+
+set_optional_property "enable-environment-properties" "$ENABLE_ENVIRONMENT_PROPERTIES"

Review Comment:
   Missed that comment.
   Fixed.
   



-- 
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] necouchman commented on a diff in pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#discussion_r1097660057


##########
guacamole-docker/bin/start.sh:
##########
@@ -1164,6 +1174,16 @@ if [ -n "$API_SESSION_TIMEOUT" ]; then
     associate_apisessiontimeout
 fi
 
+# Maximum number of bytes to accept within the entity body of any particular HTTP request
+set_optional_property "api-max-request-size" "$API_MAX_REQUEST_SIZE"
+
+# A comma-separated whitelist of language keys to allow as display language 
+# choices within the Guacamole interface
+set_optional_property "allowed-languages" "$ALLOWED_LANGUAGES"
+
+set_optional_property "enable-environment-properties" "$ENABLE_ENVIRONMENT_PROPERTIES"

Review Comment:
   This should probably have a comment associated with it.



-- 
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] necouchman commented on a diff in pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#discussion_r1108392443


##########
guacamole-docker/bin/start.sh:
##########
@@ -1164,6 +1174,19 @@ if [ -n "$API_SESSION_TIMEOUT" ]; then
     associate_apisessiontimeout
 fi
 
+# Maximum number of bytes to accept within the entity body of any particular HTTP request
+set_optional_property "api-max-request-size" "$API_MAX_REQUEST_SIZE"
+
+# A comma-separated whitelist of language keys to allow as display language 

Review Comment:
   I would suggest changing this to:
   `A comma-separated list of language keys to allow...`



-- 
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] necouchman commented on a diff in pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#discussion_r1107852630


##########
guacamole-docker/bin/start.sh:
##########
@@ -1064,6 +1069,11 @@ fi
 set_property "guacd-hostname" "$GUACD_HOSTNAME"
 set_property "guacd-port"     "$GUACD_PORT"
 
+# A comma-separated list of the identifiers of authentication providers that
+# should be allowed to fail internally without aborting the authentication process
+set_property "skip-if-unavailable"     "$SKIP_IF_UNAVAILABLE"

Review Comment:
   Shouldn't this be `set_optional_property`? Using `set_property` will put an empty string in there or possibly other unexpected results if it isn't present.



-- 
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] necouchman commented on pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #794:
URL: https://github.com/apache/guacamole-client/pull/794#issuecomment-1433847220

   Okay, the changes are good, I believe; however, the final remaining item is to clean up commits and commit messages. In particular:
   * Each commit message should be descriptive of the change in a way that helps people in the future to know the purpose of the commit (see https://www.codelord.net/2015/03/16/bad-commit-messages-hall-of-shame/).
   * Each commit message needs to be tagged with the same "GUACAMOLE-1733: " prefix at the front of it.


-- 
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] necouchman merged pull request #794: GUACAMOLE-1733: Add missing optional configuration options from docker start.sh script

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman merged PR #794:
URL: https://github.com/apache/guacamole-client/pull/794


-- 
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