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

Scale message #1487

Merged
merged 3 commits into from
Apr 11, 2017
Merged

Scale message #1487

merged 3 commits into from
Apr 11, 2017

Conversation

matush-v
Copy link
Contributor

@matush-v matush-v commented Apr 4, 2017

Adds the instance number change to the scale history message.

Also removed user from the bounce after scale message since the user already shows up in the user column

@ssalinas

@@ -102,7 +102,7 @@ private void checkReschedule(SingularityRequest newRequest, Optional<Singularity
System.currentTimeMillis(), Optional.<Boolean> absent(), newRequest.getId(), Optional.of(maybeDeployId.get()), skipHealthchecks, message, actionId, maybeBounceRequest.get().getRunShellCommandBeforeKill()));

if (createResult != SingularityCreateResult.EXISTED) {
requestManager.bounce(newRequest, System.currentTimeMillis(), user, Optional.of(String.format("Bouncing due to bounce after scale by %s", user)));
Copy link
Member

Choose a reason for hiding this comment

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

why remove the user here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's already in the user column so it seemed repetitive

@@ -581,6 +581,12 @@ public SingularityRequestParent scale(@ApiParam("The Request ID to scale") @Path
validator.checkScale(newRequest, Optional.<Integer>absent());

checkBadRequest(oldRequest.getInstancesSafe() != newRequest.getInstancesSafe(), "Scale request has no affect on the # of instances (%s)", newRequest.getInstancesSafe());
String scaleMessage = String.format("Scaling from %d instance(s) to %d instances", oldRequest.getInstancesSafe(), newRequest.getInstancesSafe());
Copy link
Member

Choose a reason for hiding this comment

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

given the size of the area it is displayed in the ui, this can probably be even shorter. i.e. from %d to %s or even %d -> %d

@ssalinas
Copy link
Member

ssalinas commented Apr 7, 2017

👍

@ssalinas ssalinas added this to the 0.15.0 milestone Apr 11, 2017
@ssalinas ssalinas merged commit 0a5ad0d into master Apr 11, 2017
@ssalinas ssalinas deleted the scale-message branch April 11, 2017 17:23
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.

2 participants