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

Product tests of Kerberized Presto CLI #4728

Closed
wants to merge 1 commit into from
Closed

Product tests of Kerberized Presto CLI #4728

wants to merge 1 commit into from

Conversation

sopel39
Copy link
Contributor

@sopel39 sopel39 commented Mar 7, 2016

No description provided.

@sopel39 sopel39 force-pushed the cli_test_kerberos branch 3 times, most recently from 91dcf90 to 2764b62 Compare March 7, 2016 14:44
@sopel39 sopel39 force-pushed the cli_test_kerberos branch from 2764b62 to e2ab147 Compare March 7, 2016 14:46
losipiuk
losipiuk reviewed Mar 8, 2016
View reviewed changes
presto-product-tests/src/main/java/com/facebook/presto/tests/cli/PrestoCliTests.java
private String serverAddress;

@Inject(optional = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

why optional?
Maybe we can force it to be either optional or not depending on list of tests to be executed?

@losipiuk
Copy link
Contributor

losipiuk commented Mar 8, 2016

So there are no specific tests for kerberos.
We just either test kerberized cli or non-kerberized depending on tempto config?
That is not super cool.

@sopel39
Copy link
Contributor Author

sopel39 commented Mar 8, 2016

We talked about it with @arhimondr. We concluded that it will be consistent with how tempto sets up Hive JDBC connection. It is either Kerberized on not depending on configuration. Anyway it is not natural to have both unsecured and secured Presto CLI running at the same time.

Additionally, we wanted to avoid hassle of always selecting/excluding kerberized/unsecured tests when running product tests. Now if you have kerberized cluster setup it would just use it with proper configuration.

@losipiuk
Copy link
Contributor

losipiuk commented Mar 8, 2016

looks good

@martint
Copy link
Contributor

martint commented May 10, 2016

I think this may be covered by #4867. Can you verify?

If not, can you rebase to resolve the merge conflicts?

@arhimondr
Copy link
Member

@martint yes, this can be closed.

@cberner cberner closed this May 10, 2016
@arhimondr arhimondr deleted the cli_test_kerberos branch May 10, 2016 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sopel39 sopel39

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sopel39 @losipiuk @martint @arhimondr @cberner @facebook-github-bot