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

api: add MinioClient.ignoreCertCheck() method #572

Merged
merged 1 commit into from
May 16, 2017

Conversation

balamurugana
Copy link
Member

This patch fixes below

  • ignoreCertCheck() enables MinioClient to ignore server certificate
    verification for HTTPS.
  • Upgrade OkHttp and Okio libraries to the latest version.

@deekoder deekoder requested review from poornas, deekoder and harshavardhana and removed request for deekoder May 12, 2017 22:24
.build();
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Readme.md should have this publicly available method documented.

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently apis doing ReST calls are documented into README.md/API.md and others are covered in javadoc.

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.

Do you want to upgrade okhttp in one PR and then do ignoreCertCheck method with the subsequent one?

@balamurugana
Copy link
Member Author

Do you want to upgrade okhttp in one PR and then do ignoreCertCheck method with the subsequent one?

upgrading okhttp patch would have no purpose. I don't like a PR with no purpose. Do you want it in that way?

This patch fixes below
* ignoreCertCheck() enables MinioClient to ignore server certificate
  verification for HTTPS.
* Upgrade OkHttp and Okio libraries to the latest version.
@harshavardhana
Copy link
Member

upgrading okhttp patch would have no purpose. I don't like a PR with no purpose. Do you want it in that way?

Yes @balamurugana if we revert this PR for some reason, it would revert the changes of migrating to okhttp3 and vice versa. So it is easier if they are separate PRs. Unless you depend on the APIs present in okhttp3 not available okhttp2 to implement ignoreCertCheck() in that case it makes sense. what do you think?

@balamurugana
Copy link
Member Author

Yes @balamurugana if we revert this PR for some reason, it would revert the changes of migrating to okhttp3 and vice versa. So it is easier if they are separate PRs. Unless you depend on the APIs present in okhttp3 not available okhttp2 to implement ignoreCertCheck() in that case it makes sense. what do you think?

okhttp2 has a slightly different way of do that.

@harshavardhana
Copy link
Member

okhttp2 has a slightly different way of do that.

Ok then i think its fine..

@harshavardhana
Copy link
Member

@poornas do you have anything to add?

Copy link
Contributor

@poornas poornas left a comment

Choose a reason for hiding this comment

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

looks good

@harshavardhana harshavardhana merged commit d268683 into minio:master May 16, 2017
@balamurugana balamurugana deleted the ignore-cert-check branch May 16, 2017 08:07
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.

3 participants