-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Remove AppVeyor #7564
Remove AppVeyor #7564
Conversation
This will allow us to have CI running concurrently on multiple PRs, since we get 30 parallel jobs on Azure Pipelines but only 1 on AppVeyor. We have parameterized --use-venv since AppVeyor was using it, but Azure Pipelines was previously not.
I like this idea -- it makes sense to drop AppVeyor. The small number of AppVeyor jobs is most evident when multiple PRs are made in a short period of time, and hampers the workflow when folks are most active. |
How do we want to do this? Disable the required AppVeyor check and be mindful of any existing PRs? I think we have few enough now that it is not a big risk. |
Sounds about right to me. :) |
I'm a little torn since Appveyor seems to have a slightly different setup than Azure (I think we've already had some PR passing on Azure but not Appveyor or the opposite) which might catch (or cause ;) ) some issues that would go undetected on Azure. But I also completely agree that Appveyor is often the bottleneck of our CI when multiple PR are updated concurrently, we could maybe reduce the Appveyor load to a single CPython version ? PS: But apparently since I worked around the discrepancy between Azure & Appveyor my argument is kinda moot so I'll follow your decision ^^ |
If we have any regret it is straightforward to revert back, but I highly suspect that we won't. :) |
I unrequired the check, and should be able to disable it once this is merged. |
Whoooops. I missed @xavfernandez's comment above (it wasn't visible when I opened this issue from the notification, since I'd been scrolled down automatically). |
No issue, let's call it fate ;) |
I just went ahead and dropped the |
I unchecked "active" for the webhook, since otherwise it was still running (and failing) on PRs, e.g. on push to #7354 after release. Feel free to change it back if there is a more graceful way to handle it that we feel like investing in. |
We might want to completely delete the webhook & uninstall the github app (if it does not impact setuptols) in a near future. |
With Azure Pipelines we get 30 parallel jobs. Currently for a single PR we're using a maximum of 7 concurrently at a time. In practice it will be less because Linux and macOS finish pretty quickly.
With AppVeyor we get 1 parallel job between pip and setuptools.
By switching all of our AppVeyor tests to Azure Pipelines, this brings our maximum concurrent jobs per PR to 9 on Azure Pipelines and 0 on AppVeyor. This means that we can have CI progressing on a minimum of 3 PRs at a time vs previously all PRs were queuing on AppVeyor.
Not to mention we now only need to focus on 3 CI providers (Travis, Azure Pipelines, GitHub) instead of 4.