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

Drop dependency on disutils #1544

Merged
merged 4 commits into from
Apr 2, 2019
Merged

Drop dependency on disutils #1544

merged 4 commits into from
Apr 2, 2019

Conversation

haizaar
Copy link
Contributor

@haizaar haizaar commented Apr 2, 2019

Good day,

Recently sanic's main code started to use distutils solely for disturils.util.strtobool function.

I use minimal python docker image (30MB) that does not include distutils. Distutils weighs 3MB which is 10% increase for the image size.
Hence I propose to implement strtobool function as part of sanic/config.py.

While distutils is part of stdlib and is expected to be present, it has a certain purpose and seeing from distutils import in main application code may look odd to some.

haizaar and others added 4 commits April 2, 2019 13:16
While distutils is part of stdlib, it feels odd to use distutils in main application code.

I personally use a (lean)[https://hub.docker.com/r/haizaar/python-minimal/tags] Python distribution for running my applications that does not include distutils.
@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #1544 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1544      +/-   ##
==========================================
- Coverage   91.45%   91.43%   -0.03%     
==========================================
  Files          18       18              
  Lines        1791     1797       +6     
  Branches      339      341       +2     
==========================================
+ Hits         1638     1643       +5     
  Misses        130      130              
- Partials       23       24       +1
Impacted Files Coverage Δ
sanic/config.py 100% <100%> (ø) ⬆️
sanic/router.py 95.43% <0%> (-0.46%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bedb22...910d49a. Read the comment docs.

Copy link
Member

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

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

I have no problem with that change.

@sjsadowski sjsadowski merged commit 0b47692 into sanic-org:master Apr 2, 2019
haizaar added a commit to zarmory/docker-python-minimal that referenced this pull request Jun 26, 2019
This reverts commit 03d05b1.

New Sanic is out which has [1] merged. We can now safely drop
distutils again.

sanic-org/sanic#1544
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.

4 participants