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

Docker improvements #12

Merged
merged 12 commits into from
Jun 5, 2020
Merged

Docker improvements #12

merged 12 commits into from
Jun 5, 2020

Conversation

jonasbn
Copy link
Collaborator

@jonasbn jonasbn commented May 31, 2020

Experimented with Alpine without success, some of the XML requirements did not work.

Also migrated to Docker image python-3.8 slim, which is quite slim.
@jonasbn jonasbn linked an issue May 31, 2020 that may be closed by this pull request
@jonasbn
Copy link
Collaborator Author

jonasbn commented May 31, 2020

I have added a branch with my Alpine work, ref:

@edumco
Copy link
Contributor

edumco commented Jun 1, 2020

I've try the docker image in a few repositories and here here my test notes:

  1. Readme could to be updated to point to the docker hub image

  2. Still needed to create a spellcheck.yml file with little information about its contents. A separate tutorial explaining in details should be created.

  3. The example on reamdme has the worlist.txt listed so when we run without creating the file an error appears.

Could be a informative message like:

You have declared a $WORDLIST_FILE_NAME file in your spellcheck but it was not found!

Remove the file declaration from spellcheck or create the file with the same name.

  1. The error message doesn't count correctly the numbers of files with errors
Misspelled words:
<htmlcontent> resources/frameworks/cucumber.md: html>body>ul>li>p
--------------------------------------------------------------------------------
UI
--------------------------------------------------------------------------------

Misspelled words:
<htmlcontent> resources/frameworks/jasmine.md: html>body>ul>li
--------------------------------------------------------------------------------
Aprenda
Javascript
Unitários
--------------------------------------------------------------------------------

!!!Spelling check failed!!!
(1) Files in repository contain spelling errors

  1. The html tags in the log might be confusing. It's not easy to transpile markdown into html using the brain :P

Later I post comments about the Dockerfile

@edumco
Copy link
Contributor

edumco commented Jun 1, 2020

My notes on the debian image

  1. Debian-slim is great!

  2. --no-install-recommends

In debian based images its a good practice to add --no-install-recommends to avoid download extra-packages. If some of these packages are really necessary they should be explicitly as a depend

RUN apt-get update \
    && apt-get install -y --no-install-recommends aspell aspell-en \
    && rm -rf /var/lib/apt/lists/*
  1. Pin pip versions using pip freeze

Its a good practice to pin the exact version used for each package and its dependences and put on a requirements.txt.

This forces the docker image build to aways produce the same result.

Today on python:3.8-slim the command pip install pyspelling && pip freeze creates the following list:

backrefs==4.3
beautifulsoup4==4.9.1
bracex==2.0
html5lib==1.0.1
lxml==4.5.1
Markdown==3.2.2
pyspelling==2.6
PyYAML==5.3.1
six==1.15.0
soupsieve==2.0.1
wcmatch==6.0.1
webencodings==0.5.1

@edumco
Copy link
Contributor

edumco commented Jun 1, 2020

About Docker alpine

1) You can have multiple Dockerfiles

Dockerfile
Dockerfile.python37
Dockerfile.python37-alpine11
Dockerfile.python38-alpine11

2) In alpine we don't need to update

Just use the --no-cache and the apk will be found download and installed without touching the cache.

Ex: apk add --no-cache package

3) Temporary package installation

Sometimes we just gonna use a package during build or installation (gcc) so there are 2 approachs:

  • Create a multistage build: Great but more complex

  • Install, use and uninstall on the same RUN: good for a single package

Ex: RUN apt install unzip && unzip myfile.zip && apt uninstall unzip

4) Building wheel for lxml takes a huge amount of time.

This doeesn't feel right. i believe it could be some bad config in lxml setup.py.

5) Alpine is so good at making small images. Dont give up!

github-action-spellcheck:alpine 106MB
github-action-spellcheck:latest 211MB

@jonasbn
Copy link
Collaborator Author

jonasbn commented Jun 1, 2020

Hi @edumco

Thanks for the elaborate feedback. I have compiled your comments to this list:

@jonasbn jonasbn self-assigned this Jun 1, 2020
@jonasbn jonasbn added the enhancement New feature or request label Jun 1, 2020
@jonasbn
Copy link
Collaborator Author

jonasbn commented Jun 5, 2020

Improvements and issues, which will be addressed separately have been migrated to separate issues. Documentation has been updated and things have been slimmed down.

Many areas could do with further improvements:

  • Documentation
  • Tutorials
  • And of course additional features

@jonasbn jonasbn merged commit 85fe37a into master Jun 5, 2020
@jonasbn jonasbn deleted the docker_improvements branch June 5, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Push image to Docker Hub or GitHub Packages
2 participants