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

Add Docker tools #943

Closed
wants to merge 1 commit into from
Closed

Add Docker tools #943

wants to merge 1 commit into from

Conversation

amancevice
Copy link

@amancevice amancevice commented Aug 11, 2018

  • Add Dockerfile with Python 2.7+3.6 interpreters and tox installed
  • Add docker-compose configuration to run tests with '-e fix-lint,py27,py36'

Run tests with docker-compose run --rm tox.

Consider publishing official Docker image that can be pulled with docker pull tox. This would allow users to run tox tests without ever needing to install tox locally. Users can mount their projects to the /tox directory inside the image and run tests in a containerized environment (as is done in docker-compose.yml)

Thanks for contributing a pull request!

If you are contributing for the first time or provide a trivial fix don't worry too
much about the checklist - we will help you get started.

Contribution checklist:

(also see CONTRIBUTING.rst for details)

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added relevant issue keyword
    in message body
  • added news fragment in changelog folder
    • fragment name: <issue number>.<type>.rst for example (588.bugfix.rst)
    • <type> is must be one of bugfix, feature, deprecation,breaking, doc, misc
    • if pr has no issue: consider creating one first or change it to the pr number after creating the pr
    • "sign" fragment with "by @"
    • please use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files - by @superuser."
    • also see examples
  • added yourself to CONTRIBUTORS (preserving alphabetical order)

@codecov
Copy link

codecov bot commented Aug 11, 2018

Codecov Report

Merging #943 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #943   +/-   ##
=======================================
  Coverage   92.85%   92.85%           
=======================================
  Files          12       12           
  Lines        2379     2379           
  Branches      413      413           
=======================================
  Hits         2209     2209           
  Misses        107      107           
  Partials       63       63

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 657b18e...1f0d419. Read the comment docs.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

I'm not a big fan of this. First we should prefer using py37 instead py36. Furthermore who's going to test/maintain this is correct, we don't have any CI tests for such. Second docker seems a much heavier requirement to introduce instead of just Python that we have as of today.

@amancevice
Copy link
Author

amancevice commented Aug 11, 2018

Using 3.7 is fine -- I was going by the contribution guidelines, which ask to run:

tox -e fix-lint,py27,py36

As for who's going to test/maintain it, I could remove the pegged dependency from the Dockerfile to be simply pip install tox. This isn't great practice in Docker, but official/automated builds on Dockerhub can be set up to produce images on every commit so you wouldn't technically need to do anything to produce new image tags; Dockerhub will just build/host them for you. That would make it pretty hands-free.

As for heavy/light I'd argue that's a bit of a matter of opinion. This isn't meant to be an additional requirement for users, but another option for installation (or "anti-installation"). Some devs prefer to work entirely in containers (many of my coworkers, for example) for a variety of reasons, one of which is not having to worry about polluting their host environments with potentially incompatible installations across projects.

At the end of the day, it's totally up to you. I can always post this image to my personal Dockerhub if it's not welcome here.

- Add Dockerfile with Python 2.7+3.6 interpreters and tox installed
- Add docker-compose configuration to run tests with '-e fix-lint,py27,py36'
@gaborbernat
Copy link
Member

I'm sort of agreeing we might want to do this; however in a fashion where we can enforce some CI rules. Until we setup docker hub I would be hesitant to accept anything. Furthermore, I would say we don't want to pass in anything under tox -e, we should just let the default envlist kick in.

Copy link
Member

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

I'm sort of agreeing we might want to do this; however in a fashion where we can enforce some CI rules. Until we setup docker hub I would be hesitant to accept anything. Furthermore, I would say we don't want to pass in anything under tox -e, we should just let the default envlist kick in.

@gaborbernat
Copy link
Member

I'll close this now, in favor of #1035. I feel we should first discuss how we want to provide and maintain this.

@gaborbernat gaborbernat closed this Oct 5, 2018
@amancevice
Copy link
Author

Sorry I haven't had the time to give this the attn it deserves. I'll add my thoughts to the issue above. Thanks for all your work on tox!

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.

2 participants