> On June 4, 2014, 11:39 a.m., Nate Cole wrote: > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/package/scripts/hive_service.py, > > line 71 > > <https://reviews.apache.org/r/22153/diff/2/?file=601950#file601950line71> > > > > Can socket.socket() throw anything?
No, it's a constructor that uses the defaults of the Socket module. The documentation doesn't indicate any exceptions. > On June 4, 2014, 11:39 a.m., Nate Cole wrote: > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/package/scripts/hive_service.py, > > line 80 > > <https://reviews.apache.org/r/22153/diff/2/?file=601950#file601950line80> > > > > I think python will auto-close out of scope; but in case of s.connect() > > failure then s.close() won't be called Sockets are automatically closed when garbage collected. In this case, I would think that since connect() failed, there's nothing to close(). If you close() a socket, you can't use it anymore. In most applications, I would expect multiple attempts on a single socket, indicating that close() isn't necessary. I suppose we could call socket.shutdown() and then socket.close() inside of the loop, but then we'd need to instantiate a new socket each time. > On June 4, 2014, 11:39 a.m., Nate Cole wrote: > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/package/scripts/hive_service.py, > > lines 89-90 > > <https://reviews.apache.org/r/22153/diff/2/?file=601950#file601950line89> > > > > I think the message of raising a Fail is printed, so no need to print > > the message first. Verify in script.py. Also whitespace ;) 10 years of habit is hard to break; removing print statement for the Fail(...). - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22153/#review44715 ----------------------------------------------------------- On June 2, 2014, 3:34 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22153/ > ----------------------------------------------------------- > > (Updated June 2, 2014, 3:34 p.m.) > > > Review request for Ambari, Nate Cole and Sid Wagle. > > > Bugs: AMBARI-5800 > https://issues.apache.org/jira/browse/AMBARI-5800 > > > Repository: ambari > > > Description > ------- > > The Hive server start check only looks for a PID when returning a successful > starting condition. Although the Hive service check depends on the server > starting, the server might not be up and running even though the PID exists. > > This changes the Hive start scripts to ensure they do not return a success > until the server because active and the connection port is open. > > > Diffs > ----- > > > ambari-server/src/main/resources/stacks/HDP/1.3.2/services/HIVE/package/scripts/hive_service.py > 568f6ec > > ambari-server/src/main/resources/stacks/HDP/2.0.6/services/HIVE/package/scripts/hive_service.py > 596eec9 > ambari-server/src/test/python/stacks/1.3.2/HIVE/test_hive_server.py 6cd9f52 > ambari-server/src/test/python/stacks/2.0.6/HIVE/test_hive_server.py cec393a > > Diff: https://reviews.apache.org/r/22153/diff/ > > > Testing > ------- > > Existing tests patched, new tests added to cover timeout period. > > ---------------------------------------------------------------------- > Total run:562 > Total errors:0 > Total failures:0 > OK > > > Thanks, > > Jonathan Hurley > >
