-
Notifications
You must be signed in to change notification settings - Fork 140
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
GitHub Actions CI config #308
Conversation
This seems sufficiently ancient as a lower bound.
The PyPy builds are failing due to our good friend Twisted #9985 [1]: main function encountered error Traceback (most recent call last): File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/defer.py", line 1529, in _cancellableInlineCallbacks _inlineCallbacks(None, g, status) File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks result = g.send(result) File ".../treq/.tox/pypy3-twisted_latest/site-packages/treq/test/local_httpbin/child.py", line 175, in _serve_tls port = yield endpoint.listen(site) File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/endpoints.py", line 1092, in listen interface=self._interface) --- <exception caught here> --- File ".../treq/.tox/pypy3-twisted_latest/site-packages/treq/test/local_httpbin/child.py", line 175, in _serve_tls port = yield endpoint.listen(site) File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/defer.py", line 122, in execute result = callable(*args, **kw) File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/posixbase.py", line 521, in listenSSL tlsFactory = tls.TLSMemoryBIOFactory(contextFactory, False, factory) File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/protocols/tls.py", line 773, in __init__ contextFactory = _ContextFactoryToConnectionFactory(contextFactory) File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/protocols/tls.py", line 651, in __init__ oldStyleContextFactory.getContext() File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/_sslverify.py", line 1678, in getContext self._context = self._makeContext() File ".../treq/.tox/pypy3-twisted_latest/site-packages/twisted/internet/_sslverify.py", line 1688, in _makeContext ctx.use_certificate(self.certificate) File ".../treq/.tox/pypy3-twisted_latest/site-packages/OpenSSL/SSL.py", line 865, in use_certificate _raise_current_error() File "/usr/lib/pypy3/lib_pypy/_functools.py", line 80, in __call__ return self._func(*(self._args + fargs), **fkeywords) File ".../treq/.tox/pypy3-twisted_latest/site-packages/OpenSSL/_util.py", line 57, in exception_from_error_queue raise exception_type(errors) OpenSSL.SSL.Error: [('SSL routines', 'SSL_CTX_use_certificate', 'ee key too small')] [1]: https://twistedmatrix.com/trac/ticket/9985
As far as I can tell this isn't sensitive? I could create a secret for it in GitHub, but then it woudn't be available to actions associated with PRs from forks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the update!
I think that Travis is dying so the migration is a must.
And GitHub Action has a nice UI/UX ,integration with GitHub and good OS coverage
.github/workflows/ci.yaml
Outdated
|
||
- run: python -m pip install tox | ||
|
||
- run: tox -q -e flake8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe run all 4 in parallel ? maybe it's faster.
- run: tox -q -e flake8 | |
- run: tox -q -p all -e flake8, towncrier, twine, check-manifest |
I think that in the future, this can be extended into an automated release to pypi job.
.github/workflows/ci.yaml
Outdated
|
||
- run: tox -e ${{ steps.pyfactor.outputs.value }}-twisted_${{ matrix.twisted-version }} | ||
|
||
- run: python -m pip install coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not install coveralls together with tox ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I'll do that.
- run: coveralls | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
COVERALLS_REPO_TOKEN: 4nctuKHTnVtbdmty2FgdO3iiWm1nb7xc4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the coveralls token public ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell we don't have much choice about it. I looked at putting it in a GitHub Actions secret, but according to the documentation it wouldn't be available on PRs from forks. The Coveralls docs don't seem to treat it like a secret, either — it's at the top of the page, not obscured at all.
IMO Coveralls and Codecov aren't really worth the trouble. I'd rather focus my time on getting treq to 100% coverage than babysit these systems, so I'm not going to worry about it until and unless it becomes a real problem.
I think that we need a robot / webhook for the whole Twisted organization to automatically update the labels after a review. Manually labels update is not fun. |
Thank you for the review @adiroiban! I've made the changes you suggested.
Yeah, that would be great to have. |
Closes #307.