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(config): respect api_server.workers #3049

Merged
merged 2 commits into from Sep 29, 2022

Conversation

judahrand
Copy link
Contributor

@judahrand judahrand commented Sep 29, 2022

What does this PR address?

bentoml serve . --production was ignoring api_server.workers in bento_configuration.yaml. This is now fixed. I believe I've maintained the previous behaviour when the value was not manually set by the user.

Fixes #(issue)

Before submitting:

@judahrand judahrand requested review from ssheng, parano and a team as code owners September 29, 2022 18:14
@judahrand judahrand requested review from larme and removed request for a team September 29, 2022 18:14
@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #3049 (3453280) into main (926b613) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3049      +/-   ##
==========================================
+ Coverage   67.96%   67.97%   +0.01%     
==========================================
  Files         113      113              
  Lines       10887    10888       +1     
  Branches     1908     1921      +13     
==========================================
+ Hits         7399     7401       +2     
  Misses       3047     3047              
+ Partials      441      440       -1     
Impacted Files Coverage Δ
bentoml/_internal/configuration/containers.py 72.94% <100.00%> (+0.50%) ⬆️

Copy link
Contributor

@sauyon sauyon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One very minor nit, but otherwise looks great, thank you!

bentoml/_internal/configuration/default_configuration.yaml Outdated Show resolved Hide resolved
@aarnphm aarnphm changed the title fix: use api_server.workers from config correctly fix(config): api_server.workers wasn't parsed correctly Sep 29, 2022
Co-authored-by: Sauyon Lee <2347889+sauyon@users.noreply.github.com>
Copy link
Member

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this

Copy link
Collaborator

@ssheng ssheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch and very lean fix. Thanks, @judahrand.

@aarnphm aarnphm changed the title fix(config): api_server.workers wasn't parsed correctly fix(config): respect api_server.workers Sep 29, 2022
@aarnphm
Copy link
Member

aarnphm commented Sep 29, 2022

I will make a follow up PR to address some pylint issue.

@aarnphm aarnphm merged commit cdcb509 into bentoml:main Sep 29, 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.

None yet

4 participants