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/09/30 15:19:02 UTC
[GitHub] [guacamole-client] jlancaster36 opened a new pull request, #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
jlancaster36 opened a new pull request, #766:
URL: https://github.com/apache/guacamole-client/pull/766
Installs unzip and fixes docker start.sh to support setting LOGBACK_LEVEL to file path without delimiter problems
--
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 pull request #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on PR #766:
URL: https://github.com/apache/guacamole-client/pull/766#issuecomment-1263936791
> Changing the sed delimiter to semicolons should prevent future conflicts
Sure, except with semicolons and backslashes, as well as any character that will be interpreted by the XML parser like an ampersand or a quote.
--
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] jlancaster36 commented on pull request #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
Posted by GitBox <gi...@apache.org>.
jlancaster36 commented on PR #766:
URL: https://github.com/apache/guacamole-client/pull/766#issuecomment-1263828705
My mistake. I assumed the LOGBACK_LEVEL values were a file path. Regardless, values containing forward slashes had them parsed out as delimiters. Changing the sed delimiter to semicolons should prevent future conflicts
--
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 #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #766:
URL: https://github.com/apache/guacamole-client/pull/766#discussion_r984734290
##########
Dockerfile:
##########
@@ -58,6 +58,9 @@ RUN apt-get update -qq \
&& apt-get install -y xmlstarlet \
&& rm -rf /var/lib/apt/lists/*
+# # Install unzip for LOGBACK_LEVEL case
Review Comment:
An extra `#` snuck in here.
##########
Dockerfile:
##########
@@ -58,6 +58,9 @@ RUN apt-get update -qq \
&& apt-get install -y xmlstarlet \
&& rm -rf /var/lib/apt/lists/*
+# # Install unzip for LOGBACK_LEVEL case
+RUN apt-get update && apt-get install unzip
Review Comment:
It would be better to combine this with the existing `apt-get` line above, which also already invokes `apt-get update`.
--
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] maglub commented on pull request #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
Posted by "maglub (via GitHub)" <gi...@apache.org>.
maglub commented on PR #766:
URL: https://github.com/apache/guacamole-client/pull/766#issuecomment-1504147422
I also just came to that conclusion, looking through the startup script. Our home built helm chart is setting the LOGBACK_LEVEL. Thanks for a quick answer.
--
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 #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #766:
URL: https://github.com/apache/guacamole-client/pull/766#issuecomment-1504111055
That it did not make it into 1.5.0, I can understand. It was probably a mistake that could easily happen, but the change is correct in the master branch. I am quite surprised, though, that nobody tried to run the docker container for a month (the one here, which is broken
@maglub It was not a mistake - the issue and fix were out of scope for 1.5.0 and went in already tagged for 1.6.0, and we did not pull it back into the 1.5.1 release. I'd like to wrap up the 1.6.0 release, soon, anyway, so hopefully it'll be out relatively soon - or we could consider pulling a couple of fixes into a 1.5.2 release. @mike-jumper ?
Also, it isn't that the container is broken, or that no one tried to run 1.5.0 or 1.5.1. It's that it is only required if you actually try to change the `LOGBACK_LEVEL` variable, which is only required if you're debugging a problem with the container.
--
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] maglub commented on pull request #766: GUACAMOLE-1628: Docker image of guacamole client is missing unzip
Posted by "maglub (via GitHub)" <gi...@apache.org>.
maglub commented on PR #766:
URL: https://github.com/apache/guacamole-client/pull/766#issuecomment-1504101091
It is quite weird that this change did not make it into neither the Dockerfile for the 1.5.0, nor 1.5.1-RC1 docker containers that are available on Dockerhub. When trying to start these containers, I get an error that `unzip` is not found.
In the Dockerfile:
```
RUN apt-get update -qq \
&& apt-get install -y xmlstarlet unzip\
&& rm -rf /var/lib/apt/lists/*
```
* The change is correct in the Master branch, but not in the staging/1.5.1 branch
* But, this `apt-get` is not yet merged into the branch `stage/1.5.1`.
* It is also not available in the tag `1.5.1-RC1` (https://github.com/apache/guacamole-client/blob/1.5.1-RC1/Dockerfile)
That it did not make it into 1.5.0, I can understand. It was probably a mistake that could easily happen, but the change is correct in the master branch. I am quite surprised, though, that nobody tried to run the docker container for a month (the one here, which is broken https://hub.docker.com/layers/guacamole/guacamole/1.5.0/images/sha256-b5e3e65ce76c6f33d12fe652133c1b945a2431bafd8ae0d7eb3c5c5883e95d2d?context=explore)
I suggest that we cherry pick the Dockerfile from master, putting it into at least the `staging/1.5.1-RC1` branch, or to create a branch `staging-1.5.0.1`, fixing it there.
--
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