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

etcdmain: Refactor etcdStart for use in embedding etcd servers #5584

Closed

Conversation

purpleidea
Copy link
Contributor

This patch refactors out the important (and often non-public) portions
of etcdStart that need to get run for use in embedding an etcd server.
This will allow projects just as mgmt (config) to embed etcd without
duplicating code, or trailing behind the latest upstream etcd changes.
It puts all of the common network connection code into a new struct...

@purpleidea
Copy link
Contributor Author

This is a new, improved cleaner take on #5432. Don't merge yet, it's still a WIP, but I wanted to push it up to run some tests...

@purpleidea purpleidea force-pushed the feat/refactor-start-struct branch 3 times, most recently from 2d10e98 to 8554631 Compare June 8, 2016 02:31
@purpleidea
Copy link
Contributor Author

Whoever... I think this is ready for review now. I'm not sure why there is a failure, but it's only on go 1.5 on one arch. Might be unrelated to this patch. I can confirm it works for me!

@purpleidea
Copy link
Contributor Author

@xiang90 I decided to write this anyways, in the hopes that you would accept as a good intermediate solution until something more permanent can be written. It is clean and easier to understand, as well as will solve most or all use cases for embedding etcd.

As well, if you like this patch, I am happy to update it with comprehensive documentation to describe this. I will also add a note to say that this interface is not to be considered stable for now, in case you'd like to change it in the future.

Thanks again for all your reviews.

@purpleidea
Copy link
Contributor Author

/cc @heyitsanthony who mentioned he'd look at this too. Thanks.

This patch refactors out the important (and often non-public) portions
of etcdStart that need to get run for use in embedding an etcd server.
This will allow projects just as `mgmt` (config) to embed etcd without
duplicating code, or trailing behind the latest upstream etcd changes.
It puts all of the common network connection code into a new struct...
@purpleidea
Copy link
Contributor Author

FWIW The tests pass on my machine once I patch https://github.com/kr/pty/issues/21#issuecomment-224487979 I have no idea what's failing, but the code does work correctly. Thanks for your consideration.

@purpleidea
Copy link
Contributor Author

@xiang90 got a chance to have a look or follow up on your idea from #5432 ? Thanks!

purpleidea added a commit to purpleidea/mgmt that referenced this pull request Jun 18, 2016
These patches are proposed upstream changes and code for and from etcd.
Ideally we would revert this patch when/if things are merged upstream!
The majority of the work is in: etcd-io/etcd#5584
@purpleidea purpleidea mentioned this pull request Jul 13, 2016
@heyitsanthony
Copy link
Contributor

Superseded by #5925. Closing

LuausDalmolin pushed a commit to LuausDalmolin/mgmt that referenced this pull request Aug 24, 2022
These patches are proposed upstream changes and code for and from etcd.
Ideally we would revert this patch when/if things are merged upstream!
The majority of the work is in: etcd-io/etcd#5584
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants