-
Notifications
You must be signed in to change notification settings - Fork 73
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
MYRIAD-239 Fixed issue with * by updating conf files, added doc #89
base: 0.2.x
Are you sure you want to change the base?
Conversation
comments, and added JUnit test case.
looks like we have a failed check based off style. Also, not sure if I believe this fixes the issue. Have you tested? |
Hey Darin, Just finished testing MYRIAD-229, moving on to this one and I'll let you --John On Fri, Aug 19, 2016 at 1:16 PM, Darin [email protected] wrote:
|
Tested yet? |
Yep, tested it on our AWS cluster. --John On Wed, Aug 24, 2016 at 12:31 PM, Darin [email protected] wrote:
|
LGTM |
@@ -212,6 +212,9 @@ public String getFrameworkName() { | |||
} | |||
|
|||
public String getFrameworkRole() { | |||
if (frameworkRole.contains("/")) { |
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.
Hi @ hokiegeek2, sorry but I think this does not solve the problem. Maybe it's because the code has changed and the patch test does not make sense with the current code. The current problem is that we are treating the default role 'asterisk' as if it were a real role and the behavior has to be as if it were not a role. When the role is 'asterisk', "frameworkRole.contains (" / ")" returns false and never executes the statement "return frameworkRole.replace (" / "," ");", then it continues to fail. I think that the error is produced by the treatment of the role 'asterisc' in "ResourceOfferContainer.setScalarValues", it would be necessary that the behavior of this function with role 'asterisk' was equal to a roleless one. What do you think?
comments, and added JUnit test case.