You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "alekseyolg (via GitHub)" <gi...@apache.org> on 2023/06/23 19:47:19 UTC
[GitHub] [superset] alekseyolg opened a new pull request, #24504: build(docker): simplification of assembly to facilitate further support
alekseyolg opened a new pull request, #24504:
URL: https://github.com/apache/superset/pull/24504
During the build of the image, a lot of unnecessary duplicate commands are passed, which are redundant. This PR allows you to correct this oversight and allows you to facilitate the process of further support by developers.
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### TESTING INSTRUCTIONS
Manually build from the Dockerfile,
```docker build -t superset ```
And the build should finish without error.
Successfully tagged superset:latest
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] alekseyolg commented on a diff in pull request #24504: build(docker): simplification of assembly to facilitate further support
Posted by "alekseyolg (via GitHub)" <gi...@apache.org>.
alekseyolg commented on code in PR #24504:
URL: https://github.com/apache/superset/pull/24504#discussion_r1240875897
##########
.github/workflows/docker.yml:
##########
@@ -50,7 +50,7 @@ jobs:
mkdir -p ./build
echo ${{ github.sha }} > ./build/SHA
echo ${{ github.event.pull_request.number }} > ./build/PR-NUM
- docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
+ DOCKER_BUILDKIT=1 docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
Review Comment:
@sebastianliebscher Hello!
Originally my code didn't include this, but without it, an error pops up that this variable is not set and a build error occurs.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sebastianliebscher commented on pull request #24504: build(docker): simplification of assembly to facilitate further support
Posted by "sebastianliebscher (via GitHub)" <gi...@apache.org>.
sebastianliebscher commented on PR #24504:
URL: https://github.com/apache/superset/pull/24504#issuecomment-1605686219
@craig-rueda @alekseyolg
GitHub runners [use buildx](https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md#tools). The tool [buildx](https://github.com/docker/buildx) is a "Docker CLI plugin for extended build capabilities with BuildKit". So there is no need to explicitly set `DOCKER_BUILDKIT=1`.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sebastianliebscher commented on a diff in pull request #24504: build(docker): simplification of assembly to facilitate further support
Posted by "sebastianliebscher (via GitHub)" <gi...@apache.org>.
sebastianliebscher commented on code in PR #24504:
URL: https://github.com/apache/superset/pull/24504#discussion_r1240870453
##########
.github/workflows/docker.yml:
##########
@@ -50,7 +50,7 @@ jobs:
mkdir -p ./build
echo ${{ github.sha }} > ./build/SHA
echo ${{ github.event.pull_request.number }} > ./build/PR-NUM
- docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
+ DOCKER_BUILDKIT=1 docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
Review Comment:
The runner already uses docker-buildx by default. Have a look [here](https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md#tools). So I don't think it is necessary setting this variable `DOCKER_BUILDKIT=1` explicitly. Please correct me if I am wrong.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda merged pull request #24504: build(docker): simplification of assembly to facilitate further support
Posted by "craig-rueda (via GitHub)" <gi...@apache.org>.
craig-rueda merged PR #24504:
URL: https://github.com/apache/superset/pull/24504
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] alekseyolg commented on a diff in pull request #24504: build(docker): simplification of assembly to facilitate further support
Posted by "alekseyolg (via GitHub)" <gi...@apache.org>.
alekseyolg commented on code in PR #24504:
URL: https://github.com/apache/superset/pull/24504#discussion_r1240875897
##########
.github/workflows/docker.yml:
##########
@@ -50,7 +50,7 @@ jobs:
mkdir -p ./build
echo ${{ github.sha }} > ./build/SHA
echo ${{ github.event.pull_request.number }} > ./build/PR-NUM
- docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
+ DOCKER_BUILDKIT=1 docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
Review Comment:
@sebastianliebscher Hello!
Originally my code didn't include this, but without it, an error pops up that this variable is not set and a build error occurs.
Please indicate each place where you think that this variable is not needed and I will try to make a commit without this variable to check.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] alekseyolg commented on a diff in pull request #24504: build(docker): simplification of assembly to facilitate further support
Posted by "alekseyolg (via GitHub)" <gi...@apache.org>.
alekseyolg commented on code in PR #24504:
URL: https://github.com/apache/superset/pull/24504#discussion_r1240875897
##########
.github/workflows/docker.yml:
##########
@@ -50,7 +50,7 @@ jobs:
mkdir -p ./build
echo ${{ github.sha }} > ./build/SHA
echo ${{ github.event.pull_request.number }} > ./build/PR-NUM
- docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
+ DOCKER_BUILDKIT=1 docker build --target ci -t ${{ github.sha }} -t "pr-${{ github.event.pull_request.number }}" .
Review Comment:
@sebastianliebscher Hello!
Originally my code didn't include this, but without it, an error pops up that this variable is not set and a build error occurs.
Apparently this feature is not used by default.
--
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@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org