Remove ThriftClientManagerProvider #60

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

andrewcox commented Feb 8, 2013

Creating the ThriftClientManager through a provider makes it harder for Guice life cycle managemers to close() the manager in a @predestroy method. The only reason it was there was to enable passing a non-default maxFrameSize.

This change makes maxFrameSize a per-client configuration parameter, and binds ThriftClientManager directly (not through a provider).

Depends on facebook/nifty#40

@andrewcox andrewcox Remove ThriftClientManagerProvider
Creating the ThriftClientManager through a provider makes it harder for Guice life cycle managemers to close() the manager in a @PreDestroy method. The only reason it was there was to enable passing a non-default maxFrameSize.

This change makes maxFrameSize a per-client configuration parameter, and binds ThriftClientManager directly (not through a provider).
5806761

This is integer max value, so nothing can be bigger than this. I think want you want here is @Min(0)

I'd replace the constant here with Integer.MAX_VALUE

Contributor

dain commented Feb 8, 2013

Looks good.

Contributor

andrewcox commented Feb 15, 2013

this has had comments addressed, and has been merged and pushed

andrewcox closed this Feb 15, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment