fishy commented on code in PR #3379:
URL: https://github.com/apache/thrift/pull/3379#discussion_r3059212919
##########
lib/go/thrift/server_socket.go:
##########
@@ -44,28 +47,75 @@ func NewTServerSocketTimeout(listenAddr string,
clientTimeout time.Duration) (*T
if err != nil {
return nil, err
}
- return &TServerSocket{addr: addr, clientTimeout: clientTimeout}, nil
+
+ return NewTServerSocketFromAddrTimeout(addr, clientTimeout), nil
}
-// Creates a TServerSocket from a net.Addr
func NewTServerSocketFromAddrTimeout(addr net.Addr, clientTimeout
time.Duration) *TServerSocket {
- return &TServerSocket{addr: addr, clientTimeout: clientTimeout}
+ factory := func(listen bool) (net.Addr, net.Listener, error) {
+ var listener net.Listener
+ var err error
+ if (listen){
+ listener, err = net.Listen(addr.Network(),
addr.String())
+ }
+ return addr, listener, err
+ }
+
+ return NewTServerSocketFromFactoryTimeout(factory, clientTimeout)
}
-func (p *TServerSocket) Listen() error {
+// Allows full customization (TLS, mocks, unix sockets, windows named pipes,
etc.)
+func NewTServerSocketFromFactoryAddrTimeout(proc func(addr net.Addr) (listener
net.Listener, err error),addr net.Addr, clientTimeout time.Duration)
*TServerSocket {
+ factory := func(listen bool) (net.Addr, net.Listener, error) {
+ var listener net.Listener
+ var err error
+ if (listen){
+ listener, err = proc(addr)
+ }
+ return addr, listener, err
+ }
+ return NewTServerSocketFromFactoryTimeout(factory, clientTimeout)
+}
+
+// Allows full customization (TLS, mocks, unix sockets, windows named pipes,
etc.)
+func NewTServerSocketFromFactoryTimeout(factory func(listen bool) (addr
net.Addr, listener net.Listener, err error), clientTimeout time.Duration)
*TServerSocket {
+ return &TServerSocket{
+ factory: factory,
+ clientTimeout: clientTimeout,
+ }
+}
+
+// --- Core methods ---
+
+func (p *TServerSocket) try_listen(raise bool) error {
p.mu.Lock()
defer p.mu.Unlock()
- if p.IsListening() {
+
+ if p.listener != nil {
+ if (raise) {
+ return NewTTransportException(ALREADY_OPEN, "Server
socket already open")
+ }
return nil
}
- l, err := net.Listen(p.addr.Network(), p.addr.String())
+
+ _, l, err := p.factory(true)
if err != nil {
return err
}
+
p.listener = l
+ p.interrupted = false
return nil
}
+func (p *TServerSocket) Open() error {
+ return p.try_listen(true)
Review Comment:
also, to mitigate the readability issues of boolean args I mentioned
earlier, when we still end up using boolean args, I'd strongly prefer to always
document the arg name at the callsite:
```suggestion
return p.try_listen(true /* raise */)
```
same for all other callsites with boolean args in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
