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

Use the certifi CA certs bundle #54

Merged
merged 1 commit into from
May 3, 2016
Merged

Conversation

njsmith
Copy link
Member

@njsmith njsmith commented Apr 21, 2016

Fixes: #53

$PYTHON $MY_DIR/manylinux1-check.py
# Make sure that SSL cert checking works
$PYTHON $MY_DIR/ssl-check.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird: this script seems to be missing from the PR. travis is green but apparently it did not reach this part of the build script, it exited after 26min (instead of more than 30min usually) with:

Broadcast message from root@testing-gce-c03d1c2f-f8aa-4f73-8fd1-b3b312ac2218
    (unknown) at 1:24 ...
The system is going down for power off NOW!

https://travis-ci.org/pypa/manylinux/builds/124631630#L3928

It's probably a preemptible VM on GCE but I think it's a failure in travis not to report this as a failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

The shutdown without restart is a bug in travis-ci/travis-ci#4924.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will restart the build manually to check that ssl-check.py is indeed missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

A similar shutdown happened again!

@ogrisel
Copy link
Contributor

ogrisel commented Apr 21, 2016

Ok I got travis to fail as expected by restarting it twice:

+ ln -s /opt/_internal/cpython-3.5.1 /opt/python/cp35-cp35m
+ rm -f Python-3.5.1.tgz
+ rm get-pip.py
+ rm -rf /usr/local/ssl
+ /pip install certifi
build_scripts/build.sh: line 62: /pip: No such file or directory
The command '/bin/sh -c bash build_scripts/build.sh && rm -r build_scripts' returned a non-zero code: 1

@njsmith
Copy link
Member Author

njsmith commented Apr 22, 2016

hopefully fixed both those issues, let's see what travis says

@ogrisel
Copy link
Contributor

ogrisel commented Apr 22, 2016

There is an import error in the ssl check python script.

@njsmith njsmith force-pushed the use-certifi branch 2 times, most recently from b12e5ca to ef4613f Compare April 27, 2016 09:53
@njsmith
Copy link
Member Author

njsmith commented Apr 28, 2016

Well... it seems to be working now, but travis timed out :-/

@njsmith
Copy link
Member Author

njsmith commented Apr 28, 2016

It looks like our last build on master took 45min 45sec, and the limit is 50min, so I don't think this is a new issue with this PR in particular, it's just that adding all those different SOABI variants has pushed us up to the limit of Travis.

I just restarted the build to see what happens, but I guess we urgently have to either split up our build (presumably into one job that builds the x86-64 image and one that builds that i686 image), or else move to a different build service.

I like the property of our current setup that we don't deploy either build unless both have passed... but given that we check that all PRs pass before merging, it probably is okay in practice (since all builds on master should be succeeding). Or alternatively we could switch to something like wercker that has support for more complex pipelines.

@ogrisel
Copy link
Contributor

ogrisel commented May 3, 2016

I like the property of our current setup that we don't deploy either build unless both have passed... but given that we check that all PRs pass before merging, it probably is okay in practice (since all builds on master should be succeeding)

+1. I think this is enough as we should not merge PRs as long as they are red.

@ogrisel ogrisel merged commit 9308fff into pypa:master May 3, 2016
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