Skip to content

Does not claim recovery is supported on docker infra#9887

Merged
mshaposhnik merged 3 commits intomasterfrom
CHE-9885
May 31, 2018
Merged

Does not claim recovery is supported on docker infra#9887
mshaposhnik merged 3 commits intomasterfrom
CHE-9885

Conversation

@mshaposhnik
Copy link
Copy Markdown
Contributor

What does this PR do?

Make Docker infra does not claim recovery is supported;
Fix in WS termination to allow execute termination after unsuccessful suspending;

What issues does this PR fix or reference?

#9885

Release Notes

N/A (bug)

Docs PR

N/A (bug)

@mshaposhnik mshaposhnik requested a review from garagatyi as a code owner May 31, 2018 09:39
@mshaposhnik mshaposhnik requested review from skabashnyuk and sleshchenko and removed request for garagatyi May 31, 2018 09:40
Copy link
Copy Markdown
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

LGTM. Please take a look my advice


@Override
public Set<RuntimeIdentity> getIdentities() throws InfrastructureException {
return containers.findIdentities();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be better to comment old line and add a link to issue why docker recovery is disabled #5814.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok

// Due to https://github.com/eclipse/che/issues/5814, recovering is not fully possible
// so infrastructure should not claim support of it.
// return containers.findIdentities();
throw new UnsupportedOperationException("Runtimes tracking currently does not supported.");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Runtimes tracking currently does not supported." should be "Runtimes tracking is not supported."

@benoitf benoitf added this to the 6.7.0 milestone Jun 4, 2018
@benoitf benoitf added the kind/bug Outline of a bug - must adhere to the bug report template. label Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Outline of a bug - must adhere to the bug report template.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants