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

Misc. Improvements and Customizations #10

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

bradhe
Copy link

@bradhe bradhe commented Nov 4, 2015

Wanted to see if there's anything you'd like to reintegrate here, and to just get and feedback you might have on approach, etc., with the new features.

I'm mostly interested in seeing the client stuff land as the existing client stuff is a little weak for (real) production use.

Features

  1. Connection pooling for RPC clients, making connections between services much more robust.
  2. Support for "void" RPC calls

Minor improvements

  1. Move everything to new protobuf upstream.

Happy to hear any feedback you might have and to pull out specific pieces. Particularly, the references to my fork (github.com/bradhe/...) could be problematic--if you're in to any of this stuff I'd be happy to issue another commit that reverts it back to mainline.

// By adding a second number here that means that we're completely shut down.
atomic.AddInt32(&c.shutdown, 1)

// Now let's close down all of the connections in the poo in the pool.
Copy link
Owner

Choose a reason for hiding this comment

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

"in the poo"? ;-)

@kylelemons
Copy link
Owner

Thanks for the interest! To start with, since Google owns this code, you'd need to sign the Google CLA before I can accept your PR. Overall the changes look reasonable, and though I'm a bit skeptical about the atomics and such in the client package, that's not a deal-breaker. I also can't pull it with your fork in the name either. Since I don't actively make use of this and can't really be trusted to do any kind of maintance, I'd also be open to adding you as a contributor to the repo so you don't have to wait crazytimes for me to check my github inbox for your pull requests...

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