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.

Sign up for GitHub

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

Jump to bottom

Migrate minio-py to use latest release 2.2.4 #80

Merged
merged 1 commit into from
Jul 6, 2017
Merged

Migrate minio-py to use latest release 2.2.4 #80

merged 1 commit into from
Jul 6, 2017

Conversation

harshavardhana
Copy link
Member

No description provided.

@nitisht nitisht requested review from krisis and nitisht July 6, 2017 00:59
@nitisht nitisht mentioned this pull request Jul 6, 2017
Closed
nitisht
nitisht reviewed Jul 6, 2017
View reviewed changes
build/py/minio-py.sh
}

installDeps() {
pip3 install --user -r ${MINIO_PY_SDK_PATH}/requirements.txt
pip3 install minio==$MINIO_PY_SDK_VERSION
}

installFunctionalTest() {
curl https://raw.githubusercontent.com/minio/minio-py/${MINIO_PY_SDK_VERSION}/tests/functional/tests.py > /usr/bin/minio-py-functional
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of curl is it a good idea to pull from a specific GitHub tag (release) so we don't get to the issue of having inconsistent tests and SDKs version.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is what its doing its picking from the tag. @nitisht

Copy link
Contributor

Choose a reason for hiding this comment

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

I missed that. Thanks @harshavardhana

nitisht
nitisht reviewed Jul 6, 2017
View reviewed changes
Copy link
Contributor

@nitisht nitisht left a comment

Choose a reason for hiding this comment

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

LGTM overall, just one comment.

@harshavardhana
Copy link
Member Author 

Upload a small file through putObject
Upload a largefile through multipart upload
Perform a server side copy of an object
Perform a server side copy of an object with pre-conditions and fail
Upload a streaming object of 1MiB
Upload a streaming object of 11MiB
Stat on the uploaded object check if it exists
Download a full object, iterate on response to save to disk
Download a full object and save locally at path
Listing using ListObjects
Listing using ListObjectsV2
Prepare for remove_objects() test.
Performing remove_objects() test.
Removing objects worked as expected.
Deleting buckets and finishing tests.

docker run --net="host" -e SERVER_ENDPOINT=10.0.0.13:9000 -e ACCESS_KEY=USWUXHGYZQYFYFFIT3RE -e SECRET_KEY=MOJRH0mkL1IPauahWITSVvyDrQbEEIwljvmxdq03 -e ENABLE_HTTPS=0 play.minio.io/mint:travis-37d6a2eb0a2b6d859d14a87c05738dbec4628fe9

@nitisht nitisht requested a review from vadmeste July 6, 2017 17:47
vadmeste
vadmeste reviewed Jul 6, 2017
View reviewed changes
build/py/install.sh Outdated
@@ -21,6 +21,7 @@ set -e
install() {
apt-get install -yq python3
apt-get install -yq python3-pip
update-alternatives --install /usr/bin/python python /usr/bin/python3.5 1
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment why this is needed ? not able to guess (is this because there is python 2 by default in ubuntu installation ?)

Copy link
Member Author

Choose a reason for hiding this comment

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

No because there is no /usr/bin/python and its needed for pretty much all python applications. Also the functional tests which we download /usr/bin/env python would fail without this.

I don't know if this needs a comment.

Copy link
Member

Choose a reason for hiding this comment

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

oh you mean python3 won't create /usr/bin/python symlink, I think you need to add a comment for this, this is not clear from the first glance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep it doesn't create.

vadmeste
vadmeste approved these changes Jul 6, 2017
View reviewed changes
Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

LGTM (one comment needed otherwise)

@nitisht
Copy link
Contributor

nitisht commented Jul 6, 2017 •
edited
Loading

Are all the comments addressed?

@harshavardhana
Copy link
Member Author

Are all the comments address?

One comment adding what @vadmeste asked.. one minute.

@harshavardhana
Migrate minio-py to use latest release 2.2.4
3ac337a 
Also fix unicode support for PYTHON by adding LANG env.

docker-library/python@dbe3e24
@nitisht nitisht merged commit b9bbd61 into minio:master Jul 6, 2017
@harshavardhana harshavardhana deleted the python branch July 6, 2017 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vadmeste vadmeste vadmeste approved these changes

@nitisht nitisht nitisht approved these changes

@krisis krisis krisis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@harshavardhana @nitisht @vadmeste @krisis