> On Feb. 10, 2017, 2:18 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java, > > lines 724-726 > > <https://reviews.apache.org/r/56540/diff/1/?file=1629358#file1629358line724> > > > > Use logging here, not System.out.println(...) > > Balázs Bence Sári wrote: > I copied existing behavior. I think the reason why it is here that the > python script checks the standard out, so the intention is to make sure this > line appears there, irrespectively of the logging config. > > Balázs Bence Sári wrote: > I reduced the occasions of System.out.println's in Ambari Server (a lot > of them existed in the original version). I am hesitating to completely > removing them as in this case the explicit intention is to make that line > appear on System.out, not the application log (as the python script relies on > the console output) and I think the most reliable way to achieve that is to > use System.out.println. > If I try to achieve that with loggers: > 1. Users may reconfigure logging, in that case the python script will > miss the output. > 2. Previously (not sure if it is still the case) Java server apps > suffered from loggers sometimes not getting flushed when System.exit() was > called.
I removed my new method containing println(). Also reduced the number of printlns() in the original code as much as possible (respecting the python script). > On Feb. 10, 2017, 2:18 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java, > > lines 715-729 > > <https://reviews.apache.org/r/56540/diff/1/?file=1629358#file1629358line715> > > > > This is strange - why not make the DatabaseConsistencyCheckHelper do > > this exception handling/logic and call it directly instead of having yet > > another method call in AmbariServer? Separation of concerns would be > > helpful. > > Balázs Bence Sári wrote: > I agree with this finding. I copied existing behavior that's why it is > here. I'll move it to DB checker. > > Balázs Bence Sári wrote: > Please have look at the new implementation. I simplified error handling > here, however not completely removed. I left DB Consistency checker's concern > to provide a check result and AmbariServer's concern to decide what to do > with the check result (e.g. exit with 1 exit code if errors occured in the > checks). However, I greatly simplified the overcomplicated logic there. I removed my new method and simplified the original check-result handling. - Balázs Bence ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56540/#review165122 ----------------------------------------------------------- On Feb. 15, 2017, 11:33 a.m., Balázs Bence Sári wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56540/ > ----------------------------------------------------------- > > (Updated Feb. 15, 2017, 11:33 a.m.) > > > Review request for Ambari, Attila Doroszlai, Attila Magyar, Jonathan Hurley, > Laszlo Puskas, Oliver Szabo, Sandor Magyari, and Sebastian Toader. > > > Bugs: AMBARI-19957 > https://issues.apache.org/jira/browse/AMBARI-19957 > > > Repository: ambari > > > Description > ------- > > Postgres allows multiple schemas on a database user's search path, that is > users can query from tables in different schemas without the need of > prefixing the tables in the query. > > This can lead to confusion when after an unsuccessful upgrade DBA's restore > the tables into a different schema (e.g. public) to Ambari's configured one. > As a result, Ambari server may see different data than indended. > > New consistency checks on server startup warn the user in such situations. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelper.java > 7aa8652 > > ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckResult.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/checks/DatabaseConsistencyChecker.java > 2aaaadd > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > 1704546 > ambari-server/src/main/python/ambari_server_main.py 7a21333 > > ambari-server/src/test/java/org/apache/ambari/server/checks/DatabaseConsistencyCheckHelperTest.java > f73562d > > Diff: https://reviews.apache.org/r/56540/diff/ > > > Testing > ------- > > - Wrote new unit tests > - Run all tests for ambari-server (all passed) > - Performed manual testing > > > Thanks, > > Balázs Bence Sári > >
