Laszlo Gaal has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23848 )

Change subject: IMPALA-14942: [tools] Impala devcontainer
......................................................................


Patch Set 25:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/Dockerfile
File .devcontainer-build/Dockerfile:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/Dockerfile@34
PS25, Line 34: America/Los_Angeles
Could this be relaxed / indirected in a way that makes it easier to customize 
for users outside of the Pacific time zone?


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/Dockerfile@110
PS25, Line 110: github.com ecdsa-sha2-nistp256 
AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAy"\
              :             
"NTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mK"\
              :             "V0U2w0WZ2YB/++Tpockg=
Shouldn't all the public keys/fingerprints published at 
https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/githubs-ssh-key-fingerprints
 be added to known_hosts?
Maybe rhe soon-to-be-retired DSA fingerprint could be dropped from the list so 
that we don't have to worry about its exact retirement date.


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/README.md
File .devcontainer-build/README.md:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/README.md@34
PS25, Line 34: neccesarry
nit: typo necessary


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/build-devcontainer.sh
File .devcontainer-build/build-devcontainer.sh:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/build-devcontainer.sh@51
PS25, Line 51: uname -p
> nit: using 'uname -m' might be more portable and future-proof
Yes; Red Hat 10 forces `uname -p` today, they dropped support for uname -p (see 
the recent toolchain changes).


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/setup-multiplatform-manifest.sh
File .devcontainer-build/setup-multiplatform-manifest.sh:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/setup-multiplatform-manifest.sh@44
PS25, Line 44: jq
jq is not installed by bootstrap_system.sh, and it is not mentioned as a 
prerequisite in the README either, so it may be missing from the user's system. 
It should be added to at least one of these locations if the scripts depend on 
it.


http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/tag-multiplatform-manifest.sh
File .devcontainer-build/tag-multiplatform-manifest.sh:

http://gerrit.cloudera.org:8080/#/c/23848/25/.devcontainer-build/tag-multiplatform-manifest.sh@44
PS25, Line 44: jq
jq presence issue as well.



--
To view, visit http://gerrit.cloudera.org:8080/23848
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6aa834494003619fcdfbd98f3bfc7c74d3e1554c
Gerrit-Change-Number: 23848
Gerrit-PatchSet: 25
Gerrit-Owner: Jason Fehr <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jason Fehr <[email protected]>
Gerrit-Reviewer: Laszlo Gaal <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Fri, 19 Jun 2026 13:10:45 +0000
Gerrit-HasComments: Yes

Reply via email to