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

Opt out of even number across racks. #949

Merged

Conversation

Calvinp
Copy link
Contributor

@Calvinp Calvinp commented Mar 14, 2016

Adds a parameter to SingularityRequest to opt out of suggesting even numbers across racks, and the UI listens to this.
Also when creating or updating requests through the UI, there is a checkbox to toggle this.

Adds a parameter to SingularityRequest to opt out of suggesting even numbers across racks, and the UI listens to this.
Also when creating or updating requests through the UI, there is a checkbox to toggle this
@Calvinp
Copy link
Contributor Author

Calvinp commented Mar 14, 2016

Possible improvements in future iterations:

  • Add a configuration for whether requests will have the hint on or off by default (when the param isn't supplied)
  • Make the checkbox not exist when 'rack sensitive' isn't checked
  • Add a checkbox to the hint box to disable future hints for this request

@ssalinas
Copy link
Member

I think the last of those nice to haves (checkbox to ignore in the future) we should implement in this PR. Should just have to get the current request json, add the field, and POST it

@Calvinp
Copy link
Contributor Author

Calvinp commented Mar 14, 2016

Working on the functionality, but how's this wording?

screenshot 2016-03-14 15 54 44

Note the fact that there are two '3' options is not a bug, it's because I cheated to get it to work with the one rack I have locally.


requestObject.rackAffinity = @getSelect2Val "#rackAffinity-#{ type }"

debugger
Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn't be committed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, editing that out.

@ssalinas
Copy link
Member

Maybe just a simpler 'Don't show this prompt again for {requestId}' in terms of messaging

@Calvinp
Copy link
Contributor Author

Calvinp commented Mar 14, 2016

Hmm I feel like it's important to specify that this applies to all users, not just the person who's clicking the button.

@ssalinas
Copy link
Member

That's fair, can add a 'for any user' to the end of the messaging, should also see if we can separate it just a bit more so it doesn't look like part of the radio buttons, the difference between the circle/square is pretty slight.

@Calvinp
Copy link
Contributor Author

Calvinp commented Mar 14, 2016

I can get the text onto one line if I don't include the specific request id (keeping it as 'this request'). Which seems fairly reasonable because the request id is in the background on this screen:

screenshot 2016-03-14 16 15 36

@Calvinp
Copy link
Contributor Author

Calvinp commented Mar 14, 2016

More separation:

screenshot 2016-03-14 16 18 12

@ssalinas
Copy link
Member

👍

Calvin Pomerantz added 2 commits March 15, 2016 13:10
…veryone for this request forever, and made the checkbox for this in the edit form field remember current state
@Calvinp
Copy link
Contributor Author

Calvinp commented Mar 15, 2016

This is now implemented. Sending this request, plus the scale request, plus the bounce request (when scaling with bounce), and ensuring none of them conflicted caused a bit of trouble, but this is resolved now.

@ssalinas ssalinas modified the milestone: 0.4.12 Mar 18, 2016
@ssalinas
Copy link
Member

Thanks for this @Calvinp , going to merge it into the the other PR branch since they go together

ssalinas added a commit that referenced this pull request Mar 21, 2016
@ssalinas ssalinas merged commit 20a68bf into even_number_across_racks Mar 21, 2016
@ssalinas ssalinas deleted the opt-out-of-even-number-across-racks branch March 21, 2016 14:47
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