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

Run our oldest supported configuration in CI #3952

Merged
merged 13 commits into from Sep 27, 2018

Conversation

Projects
None yet
2 participants
@hawkowl
Contributor

hawkowl commented Sep 25, 2018

No description provided.

hawkowl added some commits Sep 25, 2018

update some of the deps. canonicaljson needs frozendict>=1. service_i…
…dentity is 16.0+ in all supported distros. pysaml3.0.0 doesn't expose it in the __version__, but we can trust the package managers here.
fix

@hawkowl hawkowl requested a review from matrix-org/synapse-core Sep 25, 2018

@@ -64,6 +64,25 @@ setenv =
{[base]setenv}
SYNAPSE_POSTGRES = 1
[testenv:py27-old]

This comment has been minimized.

@richvdh

richvdh Sep 26, 2018

Member

doesn't this need listing under envlist ?

This comment has been minimized.

@hawkowl

hawkowl Sep 26, 2018

Contributor

The envlist is just what's run on tox with no args, so I guess not?

tox.ini Outdated
/usr/bin/find "{toxinidir}" -name '*.pyc' -delete
/bin/sh -c 'python -m synapse.python_dependencies | sed -e "s/>=/==/g" | sed -e "s/psycopg2==2.6/psycopg2==2.7/" | sed -e "s/pyopenssl==16.0.0/pyopenssl==17.0.0/" | xargs pip install'
pip install -e .
coverage run {env:COVERAGE_OPTS:} --source="{toxinidir}/synapse" \

This comment has been minimized.

@richvdh

richvdh Sep 26, 2018

Member

I wonder if we really care about the coverage here.

This comment has been minimized.

@hawkowl

hawkowl Sep 26, 2018

Contributor

Nope, not really.

tox.ini Outdated
lxml
commands =
/usr/bin/find "{toxinidir}" -name '*.pyc' -delete
/bin/sh -c 'python -m synapse.python_dependencies | sed -e "s/>=/==/g" | sed -e "s/psycopg2==2.6/psycopg2==2.7/" | sed -e "s/pyopenssl==16.0.0/pyopenssl==17.0.0/" | xargs pip install'

This comment has been minimized.

@richvdh

richvdh Sep 26, 2018

Member

could you add some comments explaining the exceptions?

This comment has been minimized.

@hawkowl

hawkowl Sep 26, 2018

Contributor

done

tox.ini Outdated
lxml
commands =
/usr/bin/find "{toxinidir}" -name '*.pyc' -delete
/bin/sh -c 'python -m synapse.python_dependencies | sed -e "s/>=/==/g" | sed -e "s/psycopg2==2.6/psycopg2==2.7/" | sed -e "s/pyopenssl==16.0.0/pyopenssl==17.0.0/" | xargs pip install'

This comment has been minimized.

@richvdh

richvdh Sep 26, 2018

Member

suggest one sed with multiple -es rather than multiple seds

This comment has been minimized.

@hawkowl

hawkowl Sep 26, 2018

Contributor

done

@@ -64,6 +64,25 @@ setenv =
{[base]setenv}
SYNAPSE_POSTGRES = 1
[testenv:py27-old]

This comment has been minimized.

@richvdh

richvdh Sep 26, 2018

Member

could you add a comment explaining what this env does (and why)?

This comment has been minimized.

@hawkowl

hawkowl Sep 26, 2018

Contributor

done

@hawkowl hawkowl requested a review from matrix-org/synapse-core Sep 26, 2018

tox.ini Outdated
commands =
/usr/bin/find "{toxinidir}" -name '*.pyc' -delete
# Make all greater-thans equals, as well as update psycopg2 and pyopenssl
# versions to older versions which still compile on current versions of

This comment has been minimized.

@richvdh

richvdh Sep 26, 2018

Member

s/older/newer/?

Could you give more specific examples? which OSes do these libs not compile on? (Travis runs on a pretty ancient os (ubuntu 14.04 ?), so I'd be surprised if there were things which we needed to support for xenial which don't compile on there)

hawkowl added some commits Sep 26, 2018

@hawkowl hawkowl requested a review from matrix-org/synapse-core Sep 27, 2018

@richvdh

lgtm otherwise

tox.ini Outdated
commands =
/usr/bin/find "{toxinidir}" -name '*.pyc' -delete
# Make all greater-thans equals so we test the oldest version of our direct
# dependencies, but make the psycopg2 version one which can compile against

This comment has been minimized.

@richvdh

richvdh Sep 27, 2018

Member

I still think we're going to stare at this in a month's time and try to remember the history here. Could you put

"although we support psycopg2 2.6 (because that's what's in xenial), the Travis build images use postgres 10, which is incompatible with 2.6, so we make 2.7 our minimum. Similarly, blah blah pyopenssl".

tox.ini Outdated
# dependencies, but make the psycopg2 version one which can compile against
# PostgreSQL 10 and the pyopenssl one which can work against an OpenSSL 1.1
# compiled cryptography.
/bin/sh -c 'python -m synapse.python_dependencies | sed -e "s/>=/==/g" -e "s/psycopg2==2.6/psycopg2==2.7/" -e "s/pyopenssl==16.0.0/pyopenssl==17.0.0/" | xargs pip install'

This comment has been minimized.

@richvdh

richvdh Sep 27, 2018

Member

[given we're not running the tests against postgres here, why do we care about psycopg2 at all?]

This comment has been minimized.

@hawkowl

hawkowl Sep 27, 2018

Contributor

I guess we could just remove it entirely, yeah.

@hawkowl hawkowl merged commit b306453 into develop Sep 27, 2018

6 checks passed

ci/circleci: sytestpy2merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy2postgresmerged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3merged Your tests passed on CircleCI!
Details
ci/circleci: sytestpy3postgresmerged Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@hawkowl hawkowl deleted the hawkowl/oldest-in-ci branch Sep 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment