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

Fix the request detail page bounce messages #1158

Merged
merged 5 commits into from
Jul 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ const RequestAlerts = ({requestId, requestAPI, bounces, activeTasksForRequest})

const requestParent = requestAPI.data;
if (bounces.length > 0 && requestParent.request) {
const runningInstanceCount = Utils.request.runningInstanceCount(activeTasksForRequest.data);
maybeBouncing = (
<Alert bsStyle="warning">
<b>Request is bouncing:</b> {runningInstanceCount} of {requestParent.request.instances} replacement tasks are currently running.
<b>Request is bouncing:</b> Attempting to start <b>{requestParent.request.instances}</b> replacement tasks.
</Alert>
);
}
Expand Down Expand Up @@ -56,7 +55,7 @@ const RequestAlerts = ({requestId, requestAPI, bounces, activeTasksForRequest})
maybeDeployProgress = (
<span>
{pendingDeployProgress}
<p>{deployingInstanceCount === instances ? ' Waiting for new tasks to become healthy.' : ''}</p>
<p>{deployingInstanceCount === instances && ' Waiting for new tasks to become healthy.'}</p>
</span>
);
} else {
Expand All @@ -74,14 +73,14 @@ const RequestAlerts = ({requestId, requestAPI, bounces, activeTasksForRequest})
}
maybeDeployProgress = (
<span>
Finished deploying {targetActiveInstances} of {instances} total instances, {nextDeployStepRemark}
Finished deploying {targetActiveInstances} total instances, {nextDeployStepRemark}
</span>
);
} else {
maybeDeployProgress = (
<span>
{
`Trying to deploy ${targetActiveInstances} of ${instances}
`Trying to deploy ${targetActiveInstances}
instances, ${deployingInstanceCount} of
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem here is that of didn't make sense. targetActiveInstances is not a fraction of instances in all cases. Instead, if the deploy is successful, it will override instances. If we want to continue mentioning instances, better wording may be Trying to deploy ${targetActiveInstances} (instead of ${instances}) instances.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks

${targetActiveInstances} new tasks are currently running.`
}
Expand Down
2 changes: 1 addition & 1 deletion SingularityUI/app/selectors/tasks.es6
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const getBouncesForRequest = (requestId) => createSelector(
[getTaskCleanups],
(taskCleanups) => (
taskCleanups.data || []).filter((tc) => (
tc.cleanupType === 'BOUNCING' && tc.taskId.requestId === requestId
tc.cleanupType === 'BOUNCING' || tc.cleanupType === 'INCREMENTAL_BOUNCE' && tc.taskId.requestId === requestId
))
);

Expand Down