Skip to content

Update Travis CI matrix #1125

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

Closed
wants to merge 2 commits into from
Closed

Update Travis CI matrix #1125

wants to merge 2 commits into from

Conversation

kevin-brown
Copy link

@kevin-brown kevin-brown commented Jan 6, 2019

  • Test Python 3.7 and Django 1.11
  • Switch to using tox-travis

@kevin-brown
Copy link
Author

Tests appear to be failing on Python 3.7 + MariaDB, and that appears to be unrelated to this pull request (which does not affect that configuration).

@kevin-brown kevin-brown changed the title [WIP] Update Travis CI matrix Update Travis CI matrix Jan 6, 2019
@matthiask
Copy link
Member

Thanks.

I have a strong preference for the Travis CI matrix without tox-travis. Compare https://travis-ci.org/jazzband/django-debug-toolbar/builds/462887366 to https://travis-ci.org/jazzband/django-debug-toolbar/builds/475893380 -- tox-travis bundles all Django versions into one build making it a bit more cumbersome to understand what's happening.

That being said, since Django 1.11.17 officially supports Python 3.7 adding this combination would be worthwhile!

@@ -1,6 +1,6 @@
[tox]
envlist =
py{27,34,35,36}-dj111
py{27,34,35,36,37}-dj111
Copy link
Member

Choose a reason for hiding this comment

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

Yes, please.


install:
- pip install tox codecov
- pip install tox tox-travis codecov
Copy link
Member

Choose a reason for hiding this comment

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

Please don't. tox-travis hides jobs for individual Django versions in the Travis CI build overview.

@kevin-brown
Copy link
Author

tox-travis bundles all Django versions into one build making it a bit more cumbersome to understand what's happening.

We both have differing ideas of how useful knowing the Django version that broke a build is on first glance.


The alternative that I generally recommend for larger projects where combining parts of the matrix would cause absurdly long individual build times (think 20 mins vs 5 mins) is to set an environment variable such as DJANGO with the Django version number that is being tested. That way Travis CI clearly shows the versions of Django + Python that are broken and tox-travis will still be able to detect it within the matrix.

You can see an example of a Travis matrix using that here: https://travis-ci.org/encode/django-rest-framework

@matthiask
Copy link
Member

OK, I didn't know that it is possible to keep separate builds for Django versions even when using tox-travis.

However, if you're doing this you're back to defining version combinations twice again, once in tox.ini and once in .travis.yml so I'm unclear how this is an improvement and/or a simplification when compared to what we're currently doing?

@matthiask
Copy link
Member

Also regarding

We both have differing ideas of how useful knowing the Django version that broke a build is on first glance.

In my* experience stuff breaks more often because of differences between Django versions than between Python versions (excluding 2.x vs. 3.x maybe, but not that much either anymore). I'm definitely not of the opinion that the Django version is less important.

  • And also with my experience.

@jdufresne
Copy link
Contributor

FWIW, I too prefer avoiding tox-travis. But that is just my opinion and I understand this may come down to one's personal preference.

@matthiask
Copy link
Member

I don't care much either way now that I learned that it is possible to keep individual builds for Django versions even with tox-travis.

I still don't see how it simplifies the configuration (since you still have to specify the combinations twice), but won't stand in the way anymore.

@matthiask
Copy link
Member

I misread what @jdufresne wrote (I didn't see "avoiding").

#1126 adds py37-dj111 to the build without adding tox-travis. Let's move forward with this.

Thanks @kevin-brown for the idea!

@matthiask matthiask closed this Jan 8, 2019
matthiask added a commit that referenced this pull request Jan 8, 2019
Fixes #1125.

Requires pinning mysqlclient<1.4 because of mysqlclient's failure to
interpolate named params into an SQL query (it converts strings to bytes
and afterwards complains that it cannot find b"first_name", e.g. in
test_raw_query_param_conversion:

  ...
  File "/home/travis/build/jazzband/django-debug-toolbar/.tox/mariadb/lib/python3.7/site-packages/MySQLdb/cursors.py", line 188, in execute
    query = query % args
  KeyError: b'first_name'
@matthiask matthiask deleted the travis-matrix branch January 8, 2019 10:15
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.

3 participants