You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "yanghua (via GitHub)" <gi...@apache.org> on 2024/03/12 02:21:41 UTC
[PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
yanghua opened a new pull request, #6166:
URL: https://github.com/apache/kyuubi/pull/6166
# :mag: Description
## Issue References ๐
<!-- Append the issue number after #. If there is no issue for you to link create one or -->
<!-- If there are no issues to link, please provide details here. -->
This pull request fixes #
## Describe Your Solution ๐ง
After running `build/dist`, the old structure is like `dist/spark-binary/spark-3.4.2-bin-hadoop3/xxx`.
In `Dockerfile`, after the command `ONBUILD COPY spark-binary ${SPARK_HOME}` takes effect. The path in the container is `/opt/spark/spark-3.4.2-bin-hadoop3`. So I suggest only copy the content of the origin spark binary, making the structure looks like `dist/spark-binary/xxx`. Finally, the path in the container (`/opt/spark/xxx`) should be correct.
## Types of changes :bookmark:
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
## Test Plan ๐งช
#### Behavior Without This Pull Request :coffin:
#### Behavior With This Pull Request :tada:
#### Related Unit Tests
---
# Checklist ๐
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "zwangsheng (via GitHub)" <gi...@apache.org>.
zwangsheng commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1990653803
Thanks, merge to master.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#discussion_r1520896712
##########
bin/docker-image-tool.sh:
##########
@@ -110,7 +110,7 @@ function build {
error "Cannot found dir SPARK_HOME $SPARK_HOME, you must configure SPARK_HOME correct."
fi
fi
- cp -r "$SPARK_HOME/" "$KYUUBI_ROOT/spark-binary/"
+ cp -r "$SPARK_HOME/." "$KYUUBI_ROOT/spark-binary/"
Review Comment:
does `"$SPARK_HOME/*"` work? it's a more common case than that dot
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "zwangsheng (via GitHub)" <gi...@apache.org>.
zwangsheng commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1989907922
Hi @yanghua, i test in my local env with following steps:
1. download Kyuubi tgz from website and tar
2. run `./bin/docker-image-tool.sh -s ~/work/spark -t test build`
3. run `docker run kyuubi:test ls -l /opt/spark`
And it's expected.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "yanghua (via GitHub)" <gi...@apache.org>.
yanghua commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1990838417
> if this is a bug fix, we should backport to branch-1.8
Do a cherry-pick?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1990715184
if this is a bug fix, we should backport to branch-1.8
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1993294990
Cherry-picked to 1.8
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "yanghua (via GitHub)" <gi...@apache.org>.
yanghua commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1990146824
> Hi @yanghua, I test in my local macOs with following steps and old script:
>
> 1. download Kyuubi tgz from website and tar
> 2. run `./bin/docker-image-tool.sh -s ~/work/spark -t test build`
> 3. run `docker run kyuubi:test ls -l /opt/spark`
>
> And it's expected. I'm not sure those issue caused by os or other problem.
>
> After all, I also test this PR in local, it fit goal. I'm fine to accept this. cc @pan3793
My steps in 1.8.1 branch:
```
./build/dist --name 1.8.1 --tgz
cd dist
sudo ./bin/docker-image-tool.sh -r xxx -t v1.8.1 build
sudo ./bin/docker-image-tool.sh -r xxx -t v1.8.1 push
```
@zwangsheng Can you try it without `-s` option?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "zwangsheng (via GitHub)" <gi...@apache.org>.
zwangsheng closed pull request #6166: [MINOR] Fix wrong copy path about SPARK_HOME in docker
URL: https://github.com/apache/kyuubi/pull/6166
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "yanghua (via GitHub)" <gi...@apache.org>.
yanghua commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1990209654
In short, if we want to copy all the contact of `src` dir to `dist`.
`cp -r src/ dist/`
`cp` command has this behavior: if `dist` exists, the result is `dist/src`
So, if we want to get the right behavior, it should be `cp -r src/. dist/` or `cp -r src/* dist/`.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "zwangsheng (via GitHub)" <gi...@apache.org>.
zwangsheng commented on PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#issuecomment-1990647511
LGTM
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [MINOR] Fix wrong copy path about SPARK_HOME in docker [kyuubi]
Posted by "yanghua (via GitHub)" <gi...@apache.org>.
yanghua commented on code in PR #6166:
URL: https://github.com/apache/kyuubi/pull/6166#discussion_r1520917795
##########
bin/docker-image-tool.sh:
##########
@@ -110,7 +110,7 @@ function build {
error "Cannot found dir SPARK_HOME $SPARK_HOME, you must configure SPARK_HOME correct."
fi
fi
- cp -r "$SPARK_HOME/" "$KYUUBI_ROOT/spark-binary/"
+ cp -r "$SPARK_HOME/." "$KYUUBI_ROOT/spark-binary/"
Review Comment:
> does `"$SPARK_HOME/*"` work? it's a more common case than that dot
oh... `*` does not work. It throws:
```
cp: cannot stat '/data00/home/yanghua/kyuubi/dist/externals/spark-3.4.2-bin-hadoop3/*': No such file or directory
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org