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

DimensionData - Handle exception when powering off a node that is already powered off #912

Merged
merged 6 commits into from Oct 24, 2016

Conversation

samuelchong
Copy link
Contributor

@samuelchong samuelchong commented Oct 17, 2016

Changes Title (replace this with a logical title for your changes)

Description

-Handle exception when powering off a node that is already stopped

  • This will be no longer throw exception if it already powered off

Status

  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

@@ -561,7 +563,7 @@ def wait_for_state(self, state, func, poll_interval=2, timeout=60, *args,
sleep(poll_interval)
cnt += 1

msg = 'Status check for object %s timed out' % (result)
msg = 'Status check for object %s timed out' % result
Copy link
Member

@Kami Kami Oct 18, 2016

Choose a reason for hiding this comment

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

For safety reasons I would leave parenthesis there - if you add another format string and format to add parenthesis there it will break during run-time so it's usually safer to just always include parenthesis (this way it's impossible to forget to add them later if and when you add another format string argument).

r = self.ex_get_node_by_id(node.id)
response_code = r.state.upper()

return response_code in ['IN_PROGRESS', 'OK', 'STOPPED', 'STOPPING']
Copy link
Member

@Kami Kami Oct 18, 2016

Choose a reason for hiding this comment

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

This looks good to me, but maybe we should consider throwing an exception if node is already in stopping / stopped state.

That's more just me thinking out loud, I'm also fine with current approach.

Copy link
Contributor Author

@samuelchong samuelchong Oct 20, 2016

Choose a reason for hiding this comment

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

Thx @Kami for your comment. My intension is to make it more user friendly; As long as the aim is achieved, doesn't matter if the state a precondition or the result of the operation, as long as it is 'STOPPED', I'm happy, no exception.

Throwing exception will means one will have to check the state every time. More code. Open for discussion.

@Kami
Copy link
Member

Kami commented Oct 18, 2016

LGTM, thanks.

@tonybaloney
Copy link
Contributor

tonybaloney commented Oct 24, 2016

👍

@asfgit asfgit merged commit 64793fd into apache:trunk Oct 24, 2016
1 check passed
asfgit pushed a commit that referenced this pull request Oct 24, 2016
asfgit pushed a commit that referenced this pull request Oct 24, 2016
@samuelchong samuelchong deleted the enhance_get_resource_state branch Oct 25, 2016
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.

None yet

4 participants