> 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.
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. > 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. 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. - Balázs Bence ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56540/#review165122 ----------------------------------------------------------- On Feb. 14, 2017, 3:27 p.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. 14, 2017, 3:27 p.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/checks/IncompatibleSchemaException.java > PRE-CREATION > > 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 > >
