Created a ticket in jira: https://issues.apache.org/jira/browse/THRIFT-825
On Thu, Jul 22, 2010 at 11:35 AM, Dmitry Demeshchuk <[email protected]> wrote: > Greetings. > > I noticed a small inconvenience in the thrift_client Erlang module. > > call(Client, Function, Args) > when is_pid(Client), is_atom(Function), is_list(Args) -> > case gen_server:call(Client, {call, Function, Args}) of > R = {ok, _} -> R; > R = {error, _} -> R; > {exception, Exception} -> throw(Exception) > end. > > The inconvenience is that we can use atoms as pointers to processes in Erlang. > > Say, I write the following wrapper: > > -module(hbase). > -export([ > start_link/0 > ]). > > start_link() -> > {ok, Client} = thrift_client:start_link("localhost", 9090, hbase_thrift), > register(hbase, Client), > {ok, Client}. > > > and then I'll want to access hbase like the following: > > thrift_client:call(hbase, getTableNames, []). > > but the guard at the code above won't allow me to use this > construction. Moreover, we may want to access a pid like this: > {Node, Pid}. > > I'd rather remove the is_pid check totally from the code and also add > a possibility to register the thrift client. > > Here's the full diff: > > --- thrift_client.erl > +++ thrift_client.erl > @@ -22,7 +22,7 @@ > -behaviour(gen_server). > > %% API > --export([start_link/2, start_link/3, start_link/4, > +-export([start_link/2, start_link/3, start_link/4, start_link/5, > start/3, start/4, > call/3, send_call/3, close/1]). > > @@ -46,6 +46,11 @@ > start_link(Host, Port, Service) when is_integer(Port), is_atom(Service) -> > start_link(Host, Port, Service, []). > > +start_link(RegisterName, Host, Port, Service, Options) -> > + {ok, Client} = start_link(Host, Port, Service, Options), > + register(RegisterName, Client), > + {ok, Client}. > + > start_link(Host, Port, Service, Options) -> > start(Host, Port, Service, [{monitor, link} | Options]). > > @@ -141,7 +146,7 @@ > end. > > call(Client, Function, Args) > - when is_pid(Client), is_atom(Function), is_list(Args) -> > + when is_atom(Function), is_list(Args) -> > case gen_server:call(Client, {call, Function, Args}) of > R = {ok, _} -> R; > R = {error, _} -> R; > @@ -149,17 +154,17 @@ > end. > > cast(Client, Function, Args) > - when is_pid(Client), is_atom(Function), is_list(Args) -> > + when is_atom(Function), is_list(Args) -> > gen_server:cast(Client, {call, Function, Args}). > > %% Sends a function call but does not read the result. This is useful > %% if you're trying to log non-oneway function calls to write-only > %% transports like thrift_disk_log_transport. > send_call(Client, Function, Args) > - when is_pid(Client), is_atom(Function), is_list(Args) -> > + when is_atom(Function), is_list(Args) -> > gen_server:call(Client, {send_call, Function, Args}). > > -close(Client) when is_pid(Client) -> > +close(Client) -> > gen_server:cast(Client, close). > > %%==================================================================== > > > Just in case, attached the diff as well but not sure if this mailing > list allows attachments. > > -- > Best regards, > Dmitry Demeshchuk > -- Best regards, Dmitry Demeshchuk
