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

core: add start()/stop() #62

Merged
merged 2 commits into from Aug 19, 2016
Merged

core: add start()/stop() #62

merged 2 commits into from Aug 19, 2016

Conversation

mfycheng
Copy link
Contributor

Summary

  • Support the ability to start and stop a container.
  • Useful for testing certain failure modes, such as what happens if you lose connection / one of your databases/backends goes down.
  • Not sure how this fits into V3. I'd be happy to open a PR into V3 if needed, I'd just have to dig around to see how it works first.

Used ZooKeeper to test Start()/Stop(), since it's already in the integration test, and was an easy way to verify that the container was stopped / restarted.

Support the ability to start and stop containers,
which is a useful way of testing certain failure
modes.
@aeneasr
Copy link
Member

aeneasr commented Aug 18, 2016

that's pretty cool, I'll review it tomorrow when I'm not so tired any more ;)

@mfycheng
Copy link
Contributor Author

Sounds good, thanks for the quick reply!

Also looking at the PR builder failure. I've narrowed it down to this commit in glide:
Masterminds/glide@102d077

Using glide at a version before that commit seems to have no issues.

@aeneasr
Copy link
Member

aeneasr commented Aug 18, 2016

Masterminds/glide#558

@mfycheng
Copy link
Contributor Author

Submitted a fix: Masterminds/glide#559

@mfycheng mfycheng closed this Aug 19, 2016
@mfycheng mfycheng reopened this Aug 19, 2016
@aeneasr
Copy link
Member

aeneasr commented Aug 19, 2016

Could you add an example for this in the README.md, so people know about the feature? You could also document best practice for dealing with this.

@aeneasr
Copy link
Member

aeneasr commented Aug 19, 2016

thank you!

@aeneasr aeneasr merged commit 0b5774e into ory:master Aug 19, 2016
@mfycheng mfycheng deleted the add-startstop branch August 19, 2016 13:41
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

2 participants