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

Resume rollouts link appears to do nothing if watch updates are slow or websocket is disconnected #1875

Closed
spadgett opened this issue Jul 24, 2017 · 1 comment
Assignees
Labels
area/usability kind/bug Categorizes issue or PR as related to a bug. priority/P2

Comments

@spadgett
Copy link
Member

The resume rollouts action on the overview and browse pages doesn't disappear on click until after we get a watch update saying that spec.paused is set to false. This means the link appears to do nothing if the websocket update doesn't happen immediately.

@smarterclayton Resume rollouts is working for me in current master for both deployments and deployment configs, so I suspect it is in fact slow watches.

@spadgett spadgett added area/usability kind/bug Categorizes issue or PR as related to a bug. priority/P2 labels Jul 24, 2017
@spadgett spadgett self-assigned this Jul 24, 2017
@benjaminapetersen
Copy link
Contributor

From what I gather here, we can trigger an alert to close immediately by returning true from the underlying callback function:

// scripts/directives/alerts.js
$scope.onClick = function(alert, link) {
  if (_.isFunction(link.onClick)) {
    // If onClick() returns true, also hide the alert.
    var close = link.onClick();   // <-- we could update this to support a promise...
    if (close) {
      alert.hidden = true;
    }
  }
};

So we would have to consider how to update this:

// services/resourceAlerts.js
// inside getPausedDeploymentAlerts()
onClick: function() {
  // we could return the promise, but if our problem is that we need the user to 
  // see something immediately this probably won't be a good solution...
  DeploymentsService.setPaused(deployment, false, {namespace: deployment.metadata.namespace}).then(
    _.noop,
    function(e) { /* stuff */});
  // or do we just return `true` to let the UI update, 
  // but surface a new error/alert if things don't actually 
  // resolve well?
}

Since the error handler for DeploymentsService.setPaused will pop a new alert, I think we can return true from the function to dismiss the immediate modal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/usability kind/bug Categorizes issue or PR as related to a bug. priority/P2
Projects
None yet
Development

No branches or pull requests

2 participants