You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sebastianliebscher (via GitHub)" <gi...@apache.org> on 2023/09/03 09:21:46 UTC

[GitHub] [superset] sebastianliebscher opened a new pull request, #25170: refactor(docker): drop `build-essential` from Docker image

sebastianliebscher opened a new pull request, #25170:
URL: https://github.com/apache/superset/pull/25170

   ### SUMMARY
   This PR practically drops the apt package `build-essential` (`gcc` etc.) from the Docker image, by removing it in the same step it got installed.
   
   Important: Users building on top of `apache/superset` would have to install these tools themselves potentially breaking their Dockerfile depending on their usage.
   
   ----
   
   Also removed because not needed:
   
   - `/var/[log,tmp]/*`: Both dirs are empty.
   - `/tmp/*`: Basically empty except for a single file from upstream Python Docker image. Removing this file wouldn't save image size.
   - `apt-get clean`: [runs automatically](https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run) for debian-based docker images.
   
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   This results in a smaller image:
   - uncompressed: `1.29 GB` down to `1.04 GB`
   - compressed: `382 MB` down to `311 MB`
   
   ### TESTING INSTRUCTIONS
   -  check if the image builds: ```docker build . -t test```
   -  run `docker-compose.yml` based on that image and check if all services are running
   - successfully run integration tests on that docker stack
   - successfully run unit test
   - check GitHub CI
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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] sfirke commented on pull request #25170: build(docker): drop `build-essential` from Docker image

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25170:
URL: https://github.com/apache/superset/pull/25170#issuecomment-1708819286

   I follow the idea and it looks nice to reduce the image size and simplify the Dockerfile!
   
   Thanks for flagging the breaking change aspect.  Because of that I wonder if this would be a good candidate for a breaking version.  Maybe something to discuss on the dev@apache email list?  The safest thing would be to insert it into a breaking change window ahead of 4.0.0 but perhaps it's okay to do this before then as it would be a few months out most likely.


-- 
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


Re: [PR] build(docker): drop `build-essential` from Docker image [superset]

Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on PR #25170:
URL: https://github.com/apache/superset/pull/25170#issuecomment-2744613346

   Yeah playing some fancy tricks here https://github.com/apache/superset/blob/master/docker/pip-install.sh#L57-L62 in the pip-install shell script to only install and delete build-essential if/when it's essential. 
   
   Now I think more and more we get the proper python wheels (pre-compiled binaries) for most python libs that have C bindings, but it's not always consistent across all libs and cpu architecture. There might be a world where we don't need build-essentials eventually, but RN it doesn't weight down our images, should only add on build time / compute.


-- 
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


Re: [PR] build(docker): drop `build-essential` from Docker image [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed pull request #25170: build(docker): drop `build-essential` from Docker image
URL: https://github.com/apache/superset/pull/25170


-- 
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


Re: [PR] build(docker): drop `build-essential` from Docker image [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #25170:
URL: https://github.com/apache/superset/pull/25170#issuecomment-2744534598

   Assuming this should be close since we never heard back on the above question. Feel free to reopen/rebase if you think it's still needed! Thanks for contributing, as always. 


-- 
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


Re: [PR] build(docker): drop `build-essential` from Docker image [superset]

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #25170:
URL: https://github.com/apache/superset/pull/25170#issuecomment-2118134668

   @sebastianliebscher hope you are well!  I see [this PR just merged ](https://github.com/apache/superset/pull/28508) and it reminded me of this one.  Should this now be closed as already accomplished via the other PR?


-- 
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