Skip to content

Update env vars to meaningful names#713

Merged
harshavardhana merged 1 commit into
minio:masterfrom
nitisht:env-update
Jun 20, 2017
Merged

Update env vars to meaningful names#713
harshavardhana merged 1 commit into
minio:masterfrom
nitisht:env-update

Conversation

@nitisht
Copy link
Copy Markdown
Contributor

@nitisht nitisht commented Jun 16, 2017

  • Changed environment var S3_ADDRESS to SERVER_ENDPOINT
  • Changed environment var S3_SECURE to ENABLE_HTTPS

harshavardhana
harshavardhana previously approved these changes Jun 16, 2017
@harshavardhana harshavardhana requested a review from krisis June 16, 2017 22:53
@krishnasrinivas
Copy link
Copy Markdown
Contributor

I think it is better to prefix the env vars by MINIO_TEST_

MINIO_TEST_ENDPOINT
MINIO_TEST_ACCESS_KEY
MINIO_TEST_SECRET_KEY
MINIO_TEST_ENABLE_HTTPS

@harshavardhana
Copy link
Copy Markdown
Member

MINIO_TEST_ENDPOINT
MINIO_TEST_ACCESS_KEY
MINIO_TEST_SECRET_KEY
MINIO_TEST_ENABLE_HTTPS

Perhaps we can use these consistently across all libraries.. 👍

Copy link
Copy Markdown
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.

Whatever name we pick for environment variables it is best defined as golang constants. I.e,

const (
serverEndpoint = "SERVER_ENDPOINT",
...
)

@nitisht
Copy link
Copy Markdown
Contributor Author

nitisht commented Jun 17, 2017

Minio SDKs will run against a target that is S3 compatible. So, I thought adding MINIO/S3 will be redundant.

@krisis yes, that will be better. I will do it

 - Changed environment var S3_SECURE to ENABLE_HTTPS
 - Added environment vars as const
@nitisht
Copy link
Copy Markdown
Contributor Author

nitisht commented Jun 17, 2017

Added constants

const (
	serverEndpoint = "SERVER_ENDPOINT"
	accessKey      = "ACCESS_KEY"
	secretKey      = "SECRET_KEY"
	enableSecurity = "ENABLE_HTTPS"
)

@harshavardhana harshavardhana merged commit ce8164b into minio:master Jun 20, 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.

4 participants