-
Notifications
You must be signed in to change notification settings - Fork 327
HAWQ-1658. Easy Hawq and PXF initialization #1396
Conversation
|
@radarwave I do believe All the above tests are passing as far as I can tell. |
@eschizoid I asked more tests because commands in start-hawq.sh would not work. Now seems they are not used by any make commands. |
Hey @radarwave I think all the PR comments were addressed and I added tests for starting |
7462e3b
to
fda0229
Compare
@eschizoid the "start-hawq.sh" script is a little bit confused, since it is more like to initiate and bootstrap Meanwhile, we may need to treat start/stop cluster which are regular maintenance actions and initiation of a cluster as two different scenarios. |
Agreed, I will make two separate
Agreed |
c3ae8e2
to
d10c5c1
Compare
@ginobiliwang @radarwave Let me know what you think about this new approach and if this satisfies all the requirements. Sorry for the wait. |
Need more error message to guide user what the correct steps to begin, 'make init-hawq' is good example, we should add error messages to other make options too. See: make hawq make start-hawq
|
Anyone can help to review on pxf side? |
EOF | ||
|
||
rm -rf ${PXF_HOME}/conf/pxf-private.classpath | ||
cat <<EOF >>${PXF_HOME}/conf/pxf-private.classpath |
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.
If https://github.com/apache/hawq/blob/master/pxf/pxf-service/src/configs/templates/pxf-private-hdp.classpath.template changes, someone who does that may forget to make the same changes in the current script.
I wonder whether it is really necessary to replace ${PXF_HOME}/conf/pxf-private.classpath
with the configuration below. If so, I would propose to generate classpath from the template (right here).
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.
@leskin-in I think we can use some inspiration from #1335. Have a make variable
with the Hadoop distro (cdh
, hdp
, emr
, etc) and load the template during build-time. That way we only have one centralized place with all the classpath templates, and each process can load them however they want. Let me know what you think.
I wonder whether it is really necessary to replace ${PXF_HOME}/conf/pxf-private.classpath with the configuration below.
Unfortunately, this is needed :(
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.
Good, I think this code does the classpath generation mentioned. 👍 for your solution.
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.
Fixed
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.
IMHO, PXF part looks fine. Thanks @eschizoid !
@radarwave I Added better error messages for all the new |
@@ -51,7 +65,10 @@ build-hawq-dev-$(OS_VERSION): $(TOP_DIR)/$(OS_VERSION)-docker/hawq-dev/Dockerfil | |||
|
|||
build-hawq-test-$(OS_VERSION): $(TOP_DIR)/$(OS_VERSION)-docker/hawq-test/Dockerfile | |||
@echo build hawq-test:$(OS_VERSION) image | |||
docker build -t hawq/hawq-test:$(OS_VERSION) $(TOP_DIR)/$(OS_VERSION)-docker/hawq-test/ | |||
docker build \ | |||
--build-arg=PXF_CLASSPATH_TEMPLATE="`cat ../../pxf/pxf-service/src/configs/templates/pxf-private-${PXF_CLASSPATH_TEMPLATE}.classpath.template`" \ |
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 might look like a hack, but I found it handy for loading files from a different docker build context. Happy to change it if there is a better way of doing this.
52b5f9f
to
e04e53c
Compare
e04e53c
to
5472ff5
Compare
Much better. +1 Thanks for making build/run hawq easier. |
@radarwave You think we can merge down this PR? |
Sure, merged. Please close this PR. |
Hey Hawq team, thanks for this great project and congrats on becoming TLP 😄
Over the last couple of days I've been using
Hawq
. However, the information on how to use/install it's not yet centralized on a single place (some bits can be found on confluence, some parts on the Pivotal official Hawq guide, and finally some bits on the Apache official Hawk site).The purpose of this PR is to provide
build init start stop status
commands for the following services:Hawq
PXF
Please let me know what you think, and if you will be interested in this. Right now I only added support for centos7 docker container, but I can quickly add the same support for centos6 images.