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

Checking for existing uProxy cloud server #379

Merged
merged 6 commits into from
Apr 6, 2016

Conversation

gitlaura
Copy link
Contributor

@gitlaura gitlaura commented Apr 5, 2016

  • Refactored common code into getDroplet_()
  • Checking for existing uproxy cloud server in start() and returning unique error if server already exists
  • Using this.waitDigitalOceanActions_() in destroyServer() to determine when the DELETE action has been completed (so that we can then create a new server if necessary)

This change is Reviewable

return Promise.reject({
'errcode': 'VM_DNE',
'message': 'Droplet ' + name + ' doesnt exist'
return Promise.resolve({
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's probably better to reject if the droplet doesn't exist; this will help simplify destroyServer_ and start. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I updated getDroplet_. Do you think this is better? Problem is that if getDroplet_ fails then start should continue but if it returns a droplet then start should throw an error. So start wasn't really simplified, but I do think it's better if getDroplet_ throws an error.

@trevj
Copy link
Contributor

trevj commented Apr 5, 2016

Nice! One comment.

* Finds a droplet with this name
* @param {String} droplet name, as a string
* @return {Promise.<Object>}, returns droplet if exists
* or undefined if it does not exist
Copy link
Contributor

Choose a reason for hiding this comment

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

update comment :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@trevj
Copy link
Contributor

trevj commented Apr 6, 2016

Better I think! A couple of suggestions then we'll be good to go.

@trevj
Copy link
Contributor

trevj commented Apr 6, 2016

👍 !

@gitlaura gitlaura merged commit efcb3ac into master Apr 6, 2016
@gitlaura gitlaura deleted the gitlaura-check-if-droplet-exists branch April 6, 2016 17: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