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

Switch to github actions #3943

Merged
merged 1 commit into from
Apr 30, 2021
Merged

Switch to github actions #3943

merged 1 commit into from
Apr 30, 2021

Conversation

davfsa
Copy link
Contributor

@davfsa davfsa commented Dec 13, 2020

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
tests/build/common_include_dir.srctree Outdated Show resolved Hide resolved
tests/pypy_bugs.txt Outdated Show resolved Hide resolved
@scoder
Copy link
Contributor

scoder commented Dec 17, 2020 via email

@davfsa
Copy link
Contributor Author

davfsa commented Jan 2, 2021

You mention that some of the tests take a /long/ time to run, do they also take a long time on travis?
You can look that up in the test logs. At the end of the test run, it lists the most long running tests, split into translation, compilation, execution times.

After a long break which i was unable to work on much, finally got back to it. The issue is that we dont get the actual architecture to run as github actions only provides amd64 runners. Something we could do is have most pipelines run on github actions (all amd64) and have other architectures run on travis.

I'm going to have a look to see if I can find another service that provides a free and fast service with builds running concurrently.

Any thoughts or ideas are welcome :)

@da-woods
Copy link
Contributor

da-woods commented Jan 3, 2021

Something we could do is have most pipelines run on github actions (all amd64) and have other architectures run on travis.

I think this might be the simplest solution for the time being.

My impression is that the non-amd64 architectures are nice but not essential. I actually wonder if a 32-bit x86 test might be more useful (since that has broken in recent memory without being caught by the test-suite). But I definitely don't think that needs to be part of this change.

@davfsa
Copy link
Contributor Author

davfsa commented Jan 3, 2021

Currently trying to figure out if its actually my fault why they take so long to run. Taking this build as an example as an example, the different architectures take 15-30mins to complete, but they seem to be taking too long on my runs.

Edit: taking a look, might be ccache as my runs don't seem to find it (travis ci probably uses the hash of the travis ci file to store it)

@davfsa
Copy link
Contributor Author

davfsa commented Jan 5, 2021

After a lot of iterations, I finally ended with this:

I have moved all the CI to github actions and the ones with different architectures run on travis ci. They take 1h to fully run on github actions and ~30min to run on travis ci (except for arm64 cpp as it seems to timeout).

Tools/ci-run.sh Outdated Show resolved Hide resolved
@davfsa
Copy link
Contributor Author

davfsa commented Feb 26, 2021

Little bump :) @scoder

@mattip
Copy link
Contributor

mattip commented Feb 28, 2021

In order to get the github actions to run, a site admin must first create a minimal github action file and commit it (Actions tab -> Simple Workflow -> click through to set up a "hello world" yml file.

@mattip
Copy link
Contributor

mattip commented Feb 28, 2021

It may be worthwhile to drop some of the runs I count ~27, including cpython3.4 and cpython3.5, and c, cpp for most of the various platforms. I think the github actions can run ~10 jobs in parallel, so if it can get to ~20 that might speed up the through-put

@da-woods
Copy link
Contributor

In order to get the github actions to run, a site admin must first create a minimal github action file and commit it (Actions tab -> Simple Workflow -> click through to set up a "hello world" yml file.

Wouldn't that be ".github/workflows/ci.yml", included in this PR? (unless I'm missing something here)

@mattip
Copy link
Contributor

mattip commented Feb 28, 2021

When I have tried this in other repos, it was not enough to simply create the file. Somehow I needed to go through the GUI and create a dummy file to trigger the mechanism.

@davfsa
Copy link
Contributor Author

davfsa commented Mar 1, 2021

When I have tried this in other repos, it was not enough to simply create the file. Somehow I needed to go through the GUI and create a dummy file to trigger the mechanism.

No, this is not needed anymore, all you need is to is enable it on the settings tab
image

Edit: When that setting is enabled, I can force push to the branch to trigger it here

@davfsa
Copy link
Contributor Author

davfsa commented Mar 1, 2021

It may be worthwhile to drop some of the runs I count ~27, including cpython3.4 and cpython3.5, and c, cpp for most of the various platforms. I think the github actions can run ~10 jobs in parallel, so if it can get to ~20 that might speed up the through-put

cpython2.7, cpython3.4 and cpython3.5 have reached their end of lifetime, so, unless the team decides to keep maintaining them, I could remove those

@scoder
Copy link
Contributor

scoder commented Mar 3, 2021 via email

@davfsa
Copy link
Contributor Author

davfsa commented Mar 3, 2021

So, just to make sure I'm understanding this correctly, I can drop tests for 3.4 and 3.5? @scoder

@maxbachmann
Copy link
Contributor

I think the github actions can run ~10 jobs in parallel, so if it can get to ~20 that might speed up the through-put

@mattip it can do 20 builds in parallel (up to 5 mac os builds) https://docs.github.com/en/actions/reference/usage-limits-billing-and-administration#usage-limits

@davfsa
Copy link
Contributor Author

davfsa commented Mar 21, 2021

Fixed merge conflicts

@mattip
Copy link
Contributor

mattip commented Mar 22, 2021

No, this is not needed anymore, all you need is to is enable it on the settings tab

Has this been done? I don't see the CI runs.

@davfsa
Copy link
Contributor Author

davfsa commented Mar 22, 2021

Has this been done? I don't see the CI runs.

As far as I'm aware, no. You can see the runs here tho: https://github.com/davfsa/cython/actions

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@scoder
Copy link
Contributor

scoder commented Apr 2, 2021

The issue is that we dont get the actual architecture to run as github actions only provides amd64 runners. Something we could do is have most pipelines run on github actions (all amd64) and have other architectures run on travis.

Can't we install a 32bit Python on the 64bit instances?

@da-woods
Copy link
Contributor

da-woods commented Apr 2, 2021

The issue is that we dont get the actual architecture to run as github actions only provides amd64 runners. Something we could do is have most pipelines run on github actions (all amd64) and have other architectures run on travis.

Can't we install a 32bit Python on the 64bit instances?

I think this problem was to do with PowerPC/Arm rather than 32-bit x86 (which we don't currently test I think)

@scoder
Copy link
Contributor

scoder commented Apr 2, 2021

32-bit x86 (which we don't currently test I think)

Ah, right. We do test it on Windows, but not on travis/Linux. Which is interesting enough. We should also test it on Linux.

Edit: I recall that we had 32bit tests at some point, though. Maybe that was on Jenkins? Anyway, we've certainly had 32bit issues in the past, so it's worth including in the CI tests.

@davfsa
Copy link
Contributor Author

davfsa commented Apr 2, 2021

The issue is that we dont get the actual architecture to run as github actions only provides amd64 runners. Something we could do is have most pipelines run on github actions (all amd64) and have other architectures run on travis.

Can't we install a 32bit Python on the 64bit instances?

You are able to select the architecture to either x64 or x86

Code taken from the README:

steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
  with:
    python-version: '3.x' # Version range or exact version of a Python version to use, using SemVer's version range syntax
    architecture: 'x64' # optional x64 or x86. Defaults to x64 if not specified
- run: python my_script.py

Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

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

Looks good to me overall.

.travis.yml Outdated Show resolved Hide resolved
Tools/ci-run.sh Outdated Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
@davfsa
Copy link
Contributor Author

davfsa commented Apr 3, 2021

@scoder Even with using pythran==0.9.5, cpp fails all the time: https://github.com/davfsa/cython/runs/2260082383 (due to different bugs)

@scoder
Copy link
Contributor

scoder commented Apr 4, 2021

due to different bugs

What I see is that pip and setuptools ended support for Py<=3.5. Meaning, we need to install older pip/setuptools versions for these.

Secondly, I didn't mean to use Pythran 0.9.5 in general but only on Py2. However, if that makes things easier to get working, I'd be ok with only installing Pythran (0.9.7) on Py3 and not at all on Py2.

@davfsa
Copy link
Contributor Author

davfsa commented Apr 4, 2021

due to different bugs

What I see is that pip and setuptools ended support for Py<=3.5. Meaning, we need to install older pip/setuptools versions for these.

Secondly, I didn't mean to use Pythran 0.9.5 in general but only on Py2. However, if that makes things easier to get working, I'd be ok with only installing Pythran (0.9.7) on Py3 and not at all on Py2.

I tried also using Pythran 0.9.7 for Py3 and Pythran 0.9.5 for Py2 but it didnt seem to work either.

Commit: https://github.com/davfsa/cython/commit/056bafd644fd27c0a54bf957685ad45999117365
Actions: https://github.com/davfsa/cython/actions/runs/714221503

@Endle
Copy link
Contributor

Endle commented Apr 7, 2021

@scoder Even with using pythran==0.9.5, cpp fails all the time: https://github.com/davfsa/cython/runs/2260082383 (due to different bugs)

I have a question - is pythran really essential for our testsuit?

@da-woods
Copy link
Contributor

da-woods commented Apr 7, 2021

I have a question - is pythran really essential for our testsuit?

Cython has an optional feature where it can generate Pythran code to try to speed up array operations. So it is necessary to have it to test that.


I wonder if we should switch in two steps:

  1. Enable github actions relatively soon (maybe after the next release?)
  2. Disable Travis a bit later (after ~1 month).

The advantage of doing it like that is that this PR wouldn't need to be in perfect working order - we could merge it with some tests failing (because we still have Travis), and it'd hopefully be easier to fix things gradually without it all having to go through @davfsa.

Please don't change this PR to do that now because I've suggested this, but I wonder if that might be easier?

@davfsa
Copy link
Contributor Author

davfsa commented Apr 7, 2021

Cython has an optional feature where it can generate Pythran code to try to speed up array operations. So it is necessary to have it to test that.

I wonder if we should switch in two steps:

1. Enable github actions relatively soon (maybe after the next release?)

2. Disable Travis a bit later (after ~1 month).

The advantage of doing it like that is that this PR wouldn't need to be in perfect working order - we could merge it with some tests failing (because we still have Travis), and it'd hopefully be easier to fix things gradually without it all having to go through @davfsa.

Please don't change this PR to do that now because I've suggested this, but I wonder if that might be easier?

I would like to get this working perfectly before just cause I'm that kind of person, lol, but if you think its best to do that, I would be fine with it. I would keep trying to fix the issue in my personal github repo and open a pr to fix it (assuming nobody has done it yet). I believe this pythran issue should be the last one before this pr being production ready.

@mattip
Copy link
Contributor

mattip commented Apr 8, 2021

I wonder if we should switch in two steps:

I think there are two parallel things needed. This PR should not remove any CI runs from the travis yml file, and a repo owner should enable github actions for the repo so we can see that the PR works here as it does in @davfsa 's fork.

@da-woods
Copy link
Contributor

da-woods commented Apr 8, 2021

I tried also using Pythran 0.9.7 for Py3 and Pythran 0.9.5 for Py2 but it didnt seem to work either.

Looking at the current master branch, it looks like for most python versions Travis isn't finding a suitable version of Pythran and it just skipping the tests. So for now that's probably the best solution

tests/build/common_include_dir.srctree Show resolved Hide resolved
Tools/ci-run.sh Outdated Show resolved Hide resolved
tests/build/common_include_dir.srctree Outdated Show resolved Hide resolved
Copy link
Contributor

@scoder scoder left a comment

Choose a reason for hiding this comment

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

The Py3.10 failure looks weird. Might be an issue on their side, but needs some investigation.

I'm really happy with this, and would love to see us switch ASAP. Should we make Py3.10 optional for now and merge this?

tests/pypy_bugs.txt Show resolved Hide resolved
Tools/ci-run.sh Outdated Show resolved Hide resolved
@davfsa
Copy link
Contributor Author

davfsa commented Apr 16, 2021

Fixed @scoder. I'm fine with merging this now. I could mark the python 3.10-dev tests as allowed_failure or just have them fail as a reminder to fix them 😛.

I have seen how some other packages emulate the different architectures on Github Actions using docker, and they seem to be faster than what I was trying, so you might see another pr finally switching off Travis CI fully. I haven't had time to play around with this and I'm not sure if I will run into the same issues as I did with what I was using, so no promises!

@scoder
Copy link
Contributor

scoder commented Apr 20, 2021

I could mark the python 3.10-dev tests as allowed_failure

That's the thing - it doesn't currently fail on travis. But then, travis has 3.10 alpha 5 and GA has alpha 7. That might make the difference here – but I don't know. So maybe there is an issue on our side and travis just can't tell us yet.

Let's mark it as allowed failure, wait for 3.10 beta to look into it, and get this merged.

@davfsa
Copy link
Contributor Author

davfsa commented Apr 20, 2021

I could mark the python 3.10-dev tests as allowed_failure

That's the thing - it doesn't currently fail on travis. But then, travis has 3.10 alpha 5 and GA has alpha 7. That might make the difference here – but I don't know. So maybe there is an issue on our side and travis just can't tell us yet.

Let's mark it as allowed failure, wait for 3.10 beta to look into it, and get this merged.

Just so you know, 3.10 beta released yesterday. For what I have seen in my personal projects, it is only available on linux and macos. I'm going to re-trigger the pipelines so you can see it

@scoder
Copy link
Contributor

scoder commented Apr 20, 2021

3.10 beta released yesterday

Erm, what? It's scheduled for May 3rd.

@davfsa
Copy link
Contributor Author

davfsa commented Apr 21, 2021

3.10 beta released yesterday

Erm, what? It's scheduled for May 3rd.

Interesting. Well, the CI seems to say it is using CPython 3.10 beta 1

image

Release: https://github.com/actions/python-versions/releases/tag/3.10.0-beta.1-105026
PR: actions/python-versions#92

Looking more into this, I found: actions/setup-python#207
It seems to actually be CPython 3.10 alpha 7 but bound to another commit.

I could force it to alpha 7 instead.

@scoder
Copy link
Contributor

scoder commented Apr 21, 2021

I could force it to alpha 7 instead.

No, this is CI, we should use whatever latest build they have.

@scoder scoder merged commit a35bd75 into cython:master Apr 30, 2021
@scoder
Copy link
Contributor

scoder commented Apr 30, 2021

Thanks a bunch. This is going to help a lot speeding up the development. I can't wait to … stop waiting for travis.

@scoder scoder added this to the 3.0 milestone Apr 30, 2021
@mattip
Copy link
Contributor

mattip commented May 30, 2021

Would it make sense to extend this to windows builds as well to consolidate things? Right now windows is on appveyor and it seems some of the failures go unnoticed. Windows on github actions can use bash shell commands, I don't know if that help since much of Tools/ci-run.sh has to do with apt and gcc.

@da-woods
Copy link
Contributor

Would it make sense to extend this to windows builds as well to consolidate things? Right now windows is on appveyor and it seems some of the failures go unnoticed.

I was wondering that too. It's definitely worth a try. I don't think anyone's particularly attached to Appveyor so if it can be made to work on Github Actions then that's probably an improvement.

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

Successfully merging this pull request may close these issues.

Move away from travis-ci
7 participants