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

Use smaller upload files for mint #41

Merged
merged 3 commits into from
Jun 28, 2017
Merged

Conversation

poornas
Copy link
Contributor

@poornas poornas commented Jun 26, 2017

Changed SDK tests to use data files from a common directory that is generated at the time of docker image build. Using smaller files to speed up execution of tests

@nitisht
Copy link
Contributor

nitisht commented Jun 26, 2017

can you remove the binary files from the commit? also move initData.sh to buildscripts dir?

@@ -16,7 +16,7 @@
#

run() {
./minio.test && ./minio.test -test.v -test.timeout 20m
./minio.go
Copy link
Member

Choose a reason for hiding this comment

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

how will this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

buildMinioGoTests() will build this binary in go-deps.sh

Copy link
Member

Choose a reason for hiding this comment

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

But why extension minio.go ? we should avoid naming it like a source file. Name it as minio-go or minio-go-test

@@ -39,7 +39,9 @@ buildInitTests() {

# Build Minio Go tests
buildMinioGoTests() {
go test -o ${minio_go_sdk_path}/minio.test -c ${minio_go_sdk_path}/api_functional_v4_test.go
go get -u github.com/sirupsen/logrus && \
Copy link
Member

Choose a reason for hiding this comment

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

Tabbing issue. please use a better editor.

mkdir $data_dir
fi
cd $data_dir
dd if=/dev/zero of=SmallFile bs=1024 count=10
Copy link
Member

Choose a reason for hiding this comment

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

Create a loop for this.. by combining common bs= values. Also use bs=1M instead of 1024 writing 1k blocks to disk is inefficient.

@nitisht
Copy link
Contributor

nitisht commented Jun 28, 2017

If there is nothing pending, can you pls approve it @harshavardhana

@nitisht
Copy link
Contributor

nitisht commented Jun 28, 2017

@poornas can you pls update the README.md with the data_dir file details, i.e.

For each file, just add these two details.

  • File name:
  • Size in MB:

This will be helpful for others adding tests cases in different SDKs

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

h

@nitisht nitisht requested review from krisis and removed request for krishnasrinivas June 28, 2017 04:10
Copy link
Member

@krisis krisis left a comment

Choose a reason for hiding this comment

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

Indentation is off in a few places. Overall the changes look good to me.

@@ -0,0 +1,31 @@
#!/bin/bash
#
# Minio Cloud Storage, (C) 2017 Minio, Inc.
Copy link
Member

Choose a reason for hiding this comment

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

The license header should say Mint instead of Minio Cloud Storage. You could fix this in a separate PR for all files.

@nitisht nitisht merged commit 6bff75b into minio:master Jun 28, 2017
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

4 participants