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

ARROW-5805: [Python] Dockerize (add to docker-compose) Python Travis CI job #4784

Closed
wants to merge 1 commit into from

Conversation

rok
Copy link
Member

@rok rok commented Jul 3, 2019

This is to resolve ARROW-5805.

@rok rok changed the title [Python] Dockerize (add to docker-compose) Python Travis CI job ARROW-5805: [Python] Dockerize (add to docker-compose) Python Travis CI job Jul 3, 2019
Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

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

@fsaintjacques can you coordinate this patch with the one you just put up for the C++ build?

cython=0.29.7
cloudpickle
hypothesis
numpy>=1.14
pandas
pytest
pytest-faulthandler
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this?

Copy link
Member

Choose a reason for hiding this comment

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

pytest=5.0 includes this now

Copy link
Member

Choose a reason for hiding this comment

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

ah, thanks

@fsaintjacques
Copy link
Contributor

fsaintjacques commented Jul 8, 2019

@wesm the cpp-ci image found in ARROW-5803 does not use conda, do you want me to convert this python image to avoid conda or tackle ARROW-5804 which is cpp with conda.

On this subject, I think we should have most tests conda-less with system/bundled dependencies, and have a single CI entry test with conda (both C++ and python). This will be more reflective on how new developers and package maintainers experience the build.

@wesm
Copy link
Member

wesm commented Jul 8, 2019

@fsaintjacques Yes I think that we should have a combined C++/Python docker-compose entry that uses dependencies from conda

@fsaintjacques
Copy link
Contributor

fsaintjacques commented Jul 8, 2019

My suggestion also imply de-confa-ying the other images (since the base arrow:cpp is relying on conda), is the community fine with this (@xhochy, @kszucs)?

Note that this patch was initially implemented replacing cpp by removing conda, I ended up using cpp-ci instead to minimize the breaking changes.

@xhochy
Copy link
Member

xhochy commented Jul 8, 2019

My suggestion also imply de-confa-ying the other images (since the base arrow:cpp is relying on conda), is the community fine with this (@xhochy, @kszucs)?

Note that this patch was initially implemented replacing cpp by removing conda, I ended up using cpp-ci instead to minimize the breaking changes.

Actually we should try to keep conda in most images as this is the ecosystem we can control best and it is our recommendation for the setup for Python users, our currently most popular user base.

@kszucs kszucs force-pushed the master branch 2 times, most recently from ed180da to 85fe336 Compare July 22, 2019 19:29
@wesm
Copy link
Member

wesm commented Oct 3, 2019

How would you all like to proceed with this patch?

@kszucs
Copy link
Member

kszucs commented Oct 3, 2019

@wesm I'm doing a pretty heavy undertaking against the docker-compose.yml
I can put up a PR tomorrow, it'd nice if you could wait for it.

@wesm
Copy link
Member

wesm commented Oct 3, 2019

No problem. I was just scanning through the patch queue

@rok
Copy link
Member Author

rok commented Oct 3, 2019

@kszucs I'd be happy to continue working on this, but I'm not clear on the direction: are we keeping conda here or not? Should we have a python image with conda and one without?

@wesm
Copy link
Member

wesm commented Nov 4, 2019

Closing as this is superseded by the GitHub Actions patch

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.

None yet

5 participants