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

Droplet State Checks #31

Merged
merged 6 commits into from
May 7, 2013
Merged

Droplet State Checks #31

merged 6 commits into from
May 7, 2013

Conversation

pearkes
Copy link
Collaborator

@pearkes pearkes commented May 5, 2013

The addition of #30 made me think of this.

Basically, during FindDroplet, we already retrieve the details of a droplet. This includes the Droplet's "status". For example, if a Droplet is shutdown it's status is off.

So, we know that some commands require certain state before working properly.

I added several middleware's: CheckDropletInactive and CheckDropletActive. This simply looks at the droplet_status injected during FindDroplet, and if it's not active/inactive, raises an error to the user.

This might be overkill and too hand-holdy. I can't really decide, but I thought I would write it anyway to show what it's like. I'm personally a bit on the fence about including it.

@pearkes
Copy link
Collaborator Author

pearkes commented May 5, 2013

Realized I don't give a concrete example above. Here's one:

$ tugboat start foobar
Droplet name provided. Finding droplet ID...done, 100823 (foobar)
Droplet must be in an 'off' state for this operation to be successful.

@blom
Copy link
Collaborator

blom commented May 5, 2013

I like it! It saves us from making an unnecessary API call and therefore reduces the time before tugboat exits. Since we get the status information for free in FindDroplet, I'm all for this. I would have worded the messages "must be on/off" instead of "must be in an on/off state", but that is just my personal preference.

sequence_restart_droplet seems to be missing use CheckDropletActive?

@pearkes
Copy link
Collaborator Author

pearkes commented May 7, 2013

Yea, that state wording is a little confusing isn't it. I'm going to correct that.

For sequence_restart_droplet, I figured because the API just starts Droplets that are off that we'd do that same. Does that make sense? Or better to be consistent?

@blom
Copy link
Collaborator

blom commented May 7, 2013

LGTM. Good point about restart, and I agree. Since DigitalOcean will start a stopped droplet here, we should probably not override that behaviour.

@pearkes
Copy link
Collaborator Author

pearkes commented May 7, 2013

Ok, cool. I'm convinced. Merging!

pearkes added a commit that referenced this pull request May 7, 2013
@pearkes pearkes merged commit 3ce1d81 into master May 7, 2013
@pearkes pearkes deleted the f-droplet-state branch May 7, 2013 15:38
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