Skip to content
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

fix: Support for a custom docker base image in dockerfile/__init__.py. #3328

Closed
wants to merge 2 commits into from

Conversation

mqk
Copy link
Contributor

@mqk mqk commented Dec 8, 2022

What does this PR address?

The 1.0.11 release broke the ability to build bentos that are based on a custom docker base image, as described in #3325. In this PR I'm trying to fix that problem. I went down the path of setting docker.distro to a sentinel value ("custom_base_image"). I have no idea if this is a palatable solution, and I won't be offended if you don't think so and prefer something else.

Fixes #3325

Before submitting:

  • Does the Pull Request follow Conventional Commits specification naming? Here are GitHub's
    guide
    on how to create a pull request.
  • Does the code follow BentoML's code style, both make format and make lint script have passed (instructions)?
    (make lint failed, but I don't think it has anything to do with these changes.)

@mqk mqk requested a review from a team as a code owner December 8, 2022 00:45
@mqk mqk requested review from sauyon and removed request for a team December 8, 2022 00:45
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Merging #3328 (0ad4888) into main (7a8ee5f) will decrease coverage by 1.57%.
The diff coverage is 50.00%.

❗ Current head 0ad4888 differs from pull request most recent head 64fc0d9. Consider uploading reports for the commit 64fc0d9 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3328      +/-   ##
==========================================
- Coverage   32.91%   31.33%   -1.58%     
==========================================
  Files         109      109              
  Lines        9917     9912       -5     
  Branches     1710     1629      -81     
==========================================
- Hits         3264     3106     -158     
- Misses       6424     6611     +187     
+ Partials      229      195      -34     
Impacted Files Coverage Δ
...internal/container/frontend/dockerfile/__init__.py 69.76% <25.00%> (-2.74%) ⬇️
src/bentoml/_internal/bento/build_config.py 50.65% <75.00%> (-4.49%) ⬇️
src/bentoml/_internal/container/__init__.py 44.79% <0.00%> (-32.30%) ⬇️
src/bentoml/_internal/utils/metrics.py 23.33% <0.00%> (-30.00%) ⬇️
src/bentoml/_internal/container/docker.py 34.48% <0.00%> (-27.59%) ⬇️
src/bentoml/_internal/container/base.py 46.72% <0.00%> (-16.83%) ⬇️
src/bentoml/_internal/io_descriptors/pandas.py 33.77% <0.00%> (-9.19%) ⬇️
src/bentoml/_internal/io_descriptors/base.py 79.54% <0.00%> (-9.10%) ⬇️
src/bentoml/_internal/bento/bento.py 73.61% <0.00%> (-6.25%) ⬇️
... and 16 more

@mqk
Copy link
Contributor Author

mqk commented Dec 8, 2022

Closing this PR, since #3329 accomplishes the same thing (but better).

@mqk mqk closed this Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: [bentoml-cli] build failed: Distro is required, got None instead.
1 participant