-
Notifications
You must be signed in to change notification settings - Fork 262
Travis-CI: Fix Container Network #906
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
Conversation
Add `--network=host` to Travis-CI PPC64le runners.
d95a7dc
to
795c100
Compare
for more information, see https://pre-commit.ci
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). |
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 ^^ |
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. |
What about wrapping this in a |
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). |
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. |
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. |
Re-starting CI |
Shouldn't we check for Travis CI, and only apply the fix for that CI system? |
Move the control of the work-around for docker on ppc64le on travis into the `docker_container` logic and do not expose as interface.
46cfd57
to
1399424
Compare
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 to me! I think the CircleCI build failure is unrelated.
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.
LGTM, thanks @ax3l
The failures are all unrelated so merging this. |
failures probably related to pypa/setuptools#3002 |
Add
--network=host
to Travis-CI PPC64le runners.Draft to fix #904
Test in openPMD/openPMD-api#1138