-
Notifications
You must be signed in to change notification settings - Fork 80
Added Framework Authentication (Issue #53). #54
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
Conversation
|
Looks awesome, thanks for the contribution! I'll give it a whirl and merge if it looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind just tidying up this block of code you've added to be consistent with the rest of the file (basically just some additional spaces around your if statements and = signs.
Thanks!
|
Would you mind rebasing this against master? Sorry it's a pain I know! |
|
No issues rebasing, but there's another PR in the queue. Which order do you want to merge them? I can probably do the rebase in the next hour, but can wait if you'd rather merge the other in first. |
|
The other PR is mine and isn't quite ready to go yet, so i'll sort that out. This one will definitely be merged first 😃 |
|
Started rebase, more merge conflicts than expected. Shouldn't be hard but don't want to rush it before a meeting expect done by EOD tomorrow. |
|
@tarnfeld rebased. Tested locally to make sure the merge went fine. Cheers. PS: If you want a maven to enforce a checkstyle I could probably add it in the next week or so. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little bit of missing whitespace here
|
Just two teeny style comments, I think we should definitely add some tools for doing checkstyle or something like that. I'm just keen to keep the code consistent across the project at the moment. If you fix those two things i'll merge the PR right away! Thanks again. |
Modified `MesosScheduler.java` and `configuration.md`. Now `mapred.mesos.framework.principal`, `mapred.mesos.framework.secretfile`, `mapred.mesos.framework.user`, and `mapred.mesos.framework.name` are configureable options. Addresses issue mesos#53 Added Support for Framework Authentication Added Support for Framework Authentication
|
@tarnfeld you caught me at a good time. Updated and squashed commits. I'll open an issue on checkstyles, I've got a few other things I'd like to contribute in the near future. Getting checkstyles will end up saving me time :). |
Added Framework Authentication (Issue #53).
Modified
MesosScheduler.javaandconfiguration.md. Nowmapred.mesos.framework.principal,mapred.mesos.framework.secretfile,mapred.mesos.framework.user, andmapred.mesos.framework.nameare configureable options.