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 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

ssalinas commented Mar 14, 2016

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

This comment has been minimized.

Copy link
@tpetr

tpetr Mar 14, 2016

Member

this shouldn't be committed

This comment has been minimized.

Copy link
@Calvinp

Calvinp Mar 14, 2016

Author Contributor

You are right, editing that out.

@ssalinas
Copy link
Member

ssalinas commented Mar 14, 2016

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

ssalinas commented Mar 14, 2016

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

ssalinas commented Mar 14, 2016

👍

Calvinp added 2 commits Mar 15, 2016
…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
@Calvinp Calvinp added the hs_stable label Mar 21, 2016
@ssalinas
Copy link
Member

ssalinas commented Mar 21, 2016

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
Opt out of even number across racks.
@ssalinas ssalinas merged commit 20a68bf into even_number_across_racks Mar 21, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ssalinas ssalinas deleted the opt-out-of-even-number-across-racks branch Mar 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.