-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(helm-chart): remove helm-chart support #110
base: main
Are you sure you want to change the base?
Conversation
@danhuawang @xunliu would you like to review this PR ? |
tweak enable-ranger option Co-authored-by: JUN <[email protected]>
@unknowntpo I will review this PR today, Thanks. |
@@ -21,7 +21,6 @@ | |||
sed -i '$d' /usr/local/sbin/start.sh | |||
sed -i '$d' /usr/local/sbin/start.sh | |||
cp /tmp/hive/core-site.xml /tmp/hadoop-conf | |||
sed -i "s|hdfs://localhost:9000|hdfs://${HIVE_HOST_IP}:9000|g" /usr/local/hive/conf/hive-site.xml | |||
/bin/bash /usr/local/sbin/start.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's placeholder in the configuration file of the hive image: https://github.com/apache/gravitino/blob/main/dev/docker/hive/hive-site.xml
I think it needs to be replaced to hdfs://hive:9000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to add this line, because in start.sh
, It has already used sed
to replace it with $(hostname)
.
sed -i "s/__REPLACE__HOST_NAME/$(hostname)/g" ${HIVE_CONF_DIR}/hive-site.xml
https://github.com/apache/gravitino/blob/branch-0.7/dev/docker/hive/start.sh#L37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can directly replace tail -f /dev/null
with empty.
Because maybe tail -f /dev/null
is not in the last line.
init/hive/init.sh
Outdated
sed -i '$d' /usr/local/sbin/start.sh | ||
sed -i '$d' /usr/local/sbin/start.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the original start.sh in image: https://github.com/apache/gravitino/blob/main/dev/docker/hive/start.sh
Is it necessary to remove the last two lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessary to remove the last two lines.
I don't get it. what do you mean ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xunliu @danhuawang fixed at ed6c5b5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to
sed -i -E 's/tail -f \/dev\/null/\s/g' /usr/local/sbin/start.sh
, since last line might be a blank string.
fixed at 7ed9dae
healthcheck/gravitino-healthcheck.sh
Outdated
while [ $attempt -lt $max_attempts ]; do | ||
response=$(curl -X GET -H "Content-Type: application/json" http://${HOST_IP}:8090/api/version) | ||
response=$(curl -X GET -H "Content-Type: application/json" http://gravitino:8090/api/version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it 127.0.0.1
as the hostname inside the gravitino container?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, reverted to 127.0.0.1
at f4404e6.
healthcheck/trino-healthcheck.sh
Outdated
@@ -20,7 +20,7 @@ | |||
set -ex | |||
|
|||
# Because trino-connector must first synchronize a default metalake from the Gravitino server | |||
response=$(trino --server ${TRINO_HOST_IP}:8080 --execute "SHOW CATALOGS LIKE 'catalog_hive'") | |||
response=$(trino --server trino:8080 --execute "SHOW CATALOGS LIKE 'catalog_hive'") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use localhost
as hostname ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, I changed this to localhost
.
1640e8f
Do we still need the following code in init/spark/init.sh ?
|
Shall we remove the placeholder "MYSQL_HOST_IP","HIVE_HOST_IP" in init/gravitino/gravitino.conf ? If removed, set env var steps in init/gravitino/init.sh is also no longer needed. |
0367525
to
badb490
Compare
ok, removed. |
ok, removed. |
1b20f74
to
7ed9dae
Compare
LGTM. |
Resolve #108