> On Aug. 22, 2016, 4:18 p.m., Jonathan Hurley wrote: > > ambari-server/src/test/python/unitTests.py, lines 207-208 > > <https://reviews.apache.org/r/51284/diff/1/?file=1480589#file1480589line207> > > > > This doesn't seem like a good test anymore.
oh no, my big fail, will remove that :( > On Aug. 22, 2016, 4:18 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql, > > line 28 > > <https://reviews.apache.org/r/51284/diff/1/?file=1480586#file1480586line28> > > > > Can you verify that this will ensure that commands run as the Ambari DB > > user are automatically scoped to the `ambari` schema? Yes, it works as expected. > On Aug. 22, 2016, 4:18 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py, lines > > 839-841 > > <https://reviews.apache.org/r/51284/diff/1/?file=1480584#file1480584line839> > > > > Why not just use the return code here to see if the command succeeded? > > Do we get a 0 return code on failed SQL statements? I think that it will return some code in case of errors, but I think that "ALTER TABLE" check is more safe than checking code. I remember that sometimes in some big scripts some of statements are failed but psql was returning 0. - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51284/#review146363 ----------------------------------------------------------- On Aug. 22, 2016, 9:39 a.m., Andrew Onischuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51284/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2016, 9:39 a.m.) > > > Review request for Ambari, Jonathan Hurley and Vitalyi Brodetskyi. > > > Bugs: AMBARI-18226 > https://issues.apache.org/jira/browse/AMBARI-18226 > > > Repository: ambari > > > Description > ------- > > There are currently two SQL files which are being used to initialized Postgres > databases. > > * > [Ambari-DDL-Postgres-CREATE.sql](https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql) > * > [Ambari-DDL-Postgres-EMBEDDED-CREATE.sql](https://github.com/apache/ambari/blob/trunk/ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql) > > There should be no need to duplicate all of the SQL DDL/DML between these two > files. It's error-prone and cumbersome to maintain. > > Instead, the problem seems to be that the embedded SQL doesn't actually switch > users after it bootstraps everything: > > > > > CREATE DATABASE :dbname; > \connect :dbname; > > ALTER ROLE :username LOGIN ENCRYPTED PASSWORD :password; > CREATE ROLE :username LOGIN ENCRYPTED PASSWORD :password; > > GRANT ALL PRIVILEGES ON DATABASE :dbname TO :username; > > CREATE SCHEMA ambari AUTHORIZATION :username; > ALTER SCHEMA ambari OWNER TO :username; > ALTER ROLE :username SET search_path TO 'ambari'; > > ------create tables and grant privileges to db user--------- > CREATE TABLE ambari.stack( > stack_id BIGINT NOT NULL, > ... > GRANT ALL PRIVILEGES ON TABLE ambari.stack TO :username; > > > This causes several problems: > > * Because tables are being creating from the `postgres` user instead of > `:username`, they need to be altered to have privileges granted. > * Because tables are being creating from the `postgres` user instead of > `:username`, the default `search_path` is wrong and needs to be prefixed to > all calls. > > Instead, the embedded SQL should leverage the remote SQL for all of the table > creation and data seeding. The embedded SQL should only be responsible for > bootstrapping the database, schema, and user. > > > Diffs > ----- > > ambari-server/src/main/python/ambari_server/dbConfiguration_linux.py > c595e41 > ambari-server/src/main/python/ambari_server/serverConfiguration.py 65bf55c > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 43bdef9 > ambari-server/src/main/resources/scripts/change_owner.sh 6f0ac3d > ambari-server/src/test/python/TestAmbariServer.py a45a4bd > ambari-server/src/test/python/unitTests.py 037b6a5 > > Diff: https://reviews.apache.org/r/51284/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Andrew Onischuk > >
