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

Travis-CI: Fix Container Network #906

Merged
merged 4 commits into from Jan 6, 2022
Merged

Conversation

ax3l
Copy link
Contributor

@ax3l ax3l commented Nov 4, 2021

Add --network=host to Travis-CI PPC64le runners.

Draft to fix #904

Test in openPMD/openPMD-api#1138

Add `--network=host` to Travis-CI PPC64le runners.
@henryiii
Copy link
Contributor

henryiii commented Nov 8, 2021

I don't think this should be added for everyone due to travis issues. I also think we need to think about how to provide a little more flexibility for the docker command (say to support podman). Maybe we could add a configuration option for controlling the command, and then you could use overrides to add it just for ppc64le?

Also, Travis's ppc64le is really a mess last I checked. Manylinux can't build using it still (maybe this is related).

@ax3l
Copy link
Contributor Author

ax3l commented Nov 11, 2021

Sounds good to me, how would you propose the control for the config option?

As an alternative, I tried again building a my project with emulation on GH actions to avoid travis, and its so slow, that I cannot get a single ppc64le build out in 360min ^^

@henryiii henryiii marked this pull request as draft November 13, 2021 16:56
@henryiii
Copy link
Contributor

I'd think some sort of option / set of options. I think it should be flexible enough to allow this and #691 too, if possible.

@joerick
Copy link
Contributor

joerick commented Nov 14, 2021

What about wrapping this in a detect_ci_provider() == CIProvider.travis_ci? I think I'd prefer that to exposing another option on our API.

@henryiii
Copy link
Contributor

I was thinking we need this anyway, #691 was about adding support for other runners (podman). If we allowed configuration here, we could also use this to allow users to work around broken Travis runners, solving both issues with one change.

The problem is Travis's, and I don't know if it's temporary or permanent (it's Travis, so it might be permanent if it it should be temporary). I like that detecting it would automatically fix it (but we should probably limit it to just the broken PPC arch, maybe just the Sao Paulo workers, as I think it's only that cluster that's broken).

@henryiii
Copy link
Contributor

Actually #340 is a better reference. The idea is that podman (and singularity, if it's close enough) are image runners that don't require sudo access. At least podman is close enough to docker that just configuring the command slightly would enable it to be used. And if we have a way to configure that generally, then we could use that to allow someone to add this flag as a workaround for Travis's issue.

@joerick
Copy link
Contributor

joerick commented Nov 16, 2021

My feeling (based on a podman fork I saw) is that a change to a different container system would require more than just changing the invocation command of the runner, it would be more like CIBW_BUILD_BACKEND - choosing between a set of container systems (E.g. CIBW_CONTAINER_RUNNER, choices docker, podman etc) - not choosing the exact command to be run. I suppose that the above could be integrated into such an option (docker-network-host), but with this being just a temporary work around, it's probably better just to get it in and worry about the API stuff later.

@ax3l ax3l marked this pull request as ready for review December 14, 2021 23:04
@ax3l ax3l changed the title [Draft] Travis-CI: Fix Container Network Travis-CI: Fix Container Network Dec 14, 2021
@ax3l
Copy link
Contributor Author

ax3l commented Dec 14, 2021

Re-starting CI

@ax3l ax3l closed this Dec 14, 2021
@ax3l ax3l reopened this Dec 14, 2021
@henryiii
Copy link
Contributor

Shouldn't we check for Travis CI, and only apply the fix for that CI system?

@henryiii henryiii mentioned this pull request Dec 21, 2021
Move the control of the work-around for docker on ppc64le on travis
into the `docker_container` logic and do not expose as interface.
Copy link
Contributor

@joerick joerick 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! I think the CircleCI build failure is unrelated.

cibuildwheel/docker_container.py Outdated Show resolved Hide resolved
Copy link
Member

@mayeut mayeut left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @ax3l

@mayeut mayeut merged commit 65c7a69 into pypa:main Jan 6, 2022
@mayeut
Copy link
Member

mayeut commented Jan 6, 2022

The failures are all unrelated so merging this.
Let's see if they also appear on main...

@mayeut
Copy link
Member

mayeut commented Jan 6, 2022

failures probably related to pypa/setuptools#3002

@ax3l ax3l deleted the fix-ppc64leNetworkTravis branch January 6, 2022 11:45
@ax3l ax3l restored the fix-ppc64leNetworkTravis branch January 6, 2022 16:04
@ax3l ax3l deleted the fix-ppc64leNetworkTravis branch January 15, 2022 22:20
@ax3l ax3l restored the fix-ppc64leNetworkTravis branch January 15, 2022 22:20
@ax3l ax3l deleted the fix-ppc64leNetworkTravis branch March 26, 2023 00:00
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.

Travis ppc64le network issue work-around
4 participants