)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"fdd4e9a58b72b8c4e99b1f6b9f91a3449a945c8f","unresolved":false,"context_lines":[{"line_number":22,"context_line":""},{"line_number":23,"context_line":"* make it work"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"  The nova/tests/functional/db/test_resource_provider.py tests will"},{"line_number":26,"context_line":"  run and pass with the \u0027placement\u0027 database fixture selected."},{"line_number":27,"context_line":"  However it does a migration, and when the placement database is"},{"line_number":28,"context_line":"  just the api database it should not."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bacf61ea_48bd6667","line":25,"updated":"2016-07-29 13:30:02.000000000","message":"So due to the way how our database fixture is implemented and that we use an SQLite in-memory DB connection as is we are always using two different databases for api and placement configs accordingly, so it must be ok to run the migration scripts.\n\nAt the same time it means, that unit tests will always be run for a configuration when these databases are separate. Not sure we wanted to test both here, though.","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"fdd4e9a58b72b8c4e99b1f6b9f91a3449a945c8f","unresolved":false,"context_lines":[{"line_number":37,"context_line":"  things are needed. We want the models to be the same, just"},{"line_number":38,"context_line":"  different databases. As long as the right tables are created via"},{"line_number":39,"context_line":"  the right migrations (api when placement connection is not"},{"line_number":40,"context_line":"  configure, placement when it is) the same models ought to suffice."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"  Given that I\u0027m not sure the approach I\u0027ve started is at all"},{"line_number":43,"context_line":"  correct."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bacf61ea_283592f6","line":40,"updated":"2016-07-29 13:30:02.000000000","message":"That\u0027s correct. Actually, we don\u0027t even need the models to be separated right now. We could reuse the ones from the api metadata, as these tables do not have any references to other nova tables.\n\nIt must be a better of using the new instance of enginefacade everywhere and then configuring it in both tests and production code with correct connection strings.","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"}],"nova/api/openstack/placement/manage.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"df499bbfd047062a9132ff8f06465ef3f435d313","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        return parser"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def take_action(self, parsed_args):"},{"line_number":35,"context_line":"        eng \u003d engines.create_engine(self.config.database.connection,"},{"line_number":36,"context_line":"                                    mysql_sql_mode\u003d\u0027TRADITIONAL\u0027)"},{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            migration.db_sync(eng, parsed_args.db_version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_fb4ce271","line":35,"updated":"2016-07-29 13:37:09.000000000","message":"(notes to myself) placement api has its own config file, so this should be ok - we don\u0027t need any fallbacks to api db here.","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"7e430c80cb19030294fb781c108c88e34392dfa1","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        return parser"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def take_action(self, parsed_args):"},{"line_number":35,"context_line":"        eng \u003d engines.create_engine(self.config.database.connection,"},{"line_number":36,"context_line":"                                    mysql_sql_mode\u003d\u0027TRADITIONAL\u0027)"},{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            migration.db_sync(eng, parsed_args.db_version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_7e5900fe","line":35,"in_reply_to":"bacf61ea_9e0f942d","updated":"2016-07-29 13:57:32.000000000","message":"got it, then we\u0027ll need to fix this to work with the fallback to api connection string and look for the correct group/option in the first place","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2b0d3866cc41e2579aff3fea40edd12a869046a4","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        return parser"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def take_action(self, parsed_args):"},{"line_number":35,"context_line":"        eng \u003d engines.create_engine(self.config.database.connection,"},{"line_number":36,"context_line":"                                    mysql_sql_mode\u003d\u0027TRADITIONAL\u0027)"},{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            migration.db_sync(eng, parsed_args.db_version)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_9e0f942d","line":35,"in_reply_to":"bacf61ea_fb4ce271","updated":"2016-07-29 13:50:04.000000000","message":"we\u0027re not going to be doing the separate conf file thing, just a separate, optional, connection setting in config","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"}],"nova/conf/database.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"02cbcaed1ac2f44214e97fec4cb37481c1ebc4b7","unresolved":false,"context_lines":[{"line_number":100,"context_line":"placement_db_opts \u003d ["},{"line_number":101,"context_line":"    cfg.StrOpt(\u0027connection\u0027,"},{"line_number":102,"context_line":"               default\u003dNone,"},{"line_number":103,"context_line":"               help\u003d\u0027The SQLAlchemy connection string to use to connect to \u0027"},{"line_number":104,"context_line":"                    \u0027the Placement API database. If None, the API database is \u0027"},{"line_number":105,"context_line":"                    \u0027used.\u0027,"},{"line_number":106,"context_line":"               secret\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":2,"id":"bacf61ea_47c9c825","line":103,"updated":"2016-07-29 16:59:52.000000000","message":"I know that the existing options haven\u0027t been updated yet, but I think that all new options should follow the consistency guidelines:\n\nhttps://wiki.openstack.org/wiki/ConfigOptionsConsistency","commit_id":"d5294f8da9c61d1e4043b3a0e77cc10c8d6c686d"}],"nova/db/placement/models.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"df499bbfd047062a9132ff8f06465ef3f435d313","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"from oslo_db.sqlalchemy import models"},{"line_number":15,"context_line":"from sqlalchemy import Column"},{"line_number":16,"context_line":"from sqlalchemy.ext.declarative import declarative_base"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_db8c7e81","line":13,"updated":"2016-07-29 13:37:09.000000000","message":"As I stated in the other comment we can keep these or just the models from api metadata instead - it must not matter, as long as we migrations / models definitions for both database are in sync and we configure the enginefacade instance on start with the correct connection string value","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"df499bbfd047062a9132ff8f06465ef3f435d313","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"def get_placement_engine():"},{"line_number":153,"context_line":"    return placement_context_manager.get_legacy_facade().get_engine()"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"_SHADOW_TABLE_PREFIX \u003d \u0027shadow_\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_5b2a4e2d","line":153,"updated":"2016-07-29 13:37:09.000000000","message":"(notes to myself) in tests (!) at this point it\u0027s another connection, even if placement connection value is not set and we default to api one (sqlite:// - in-memory DB)","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"}],"nova/tests/fixtures.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"79af0f5e517c50a069837f40adc8de61c095032f","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        elif database \u003d\u003d \u0027api\u0027:"},{"line_number":244,"context_line":"            self.get_engine \u003d session.get_api_engine"},{"line_number":245,"context_line":"        elif database \u003d\u003d \u0027placement\u0027:"},{"line_number":246,"context_line":"            self.get_engine \u003d session.get_placement_engine"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _cache_schema(self):"},{"line_number":249,"context_line":"        global DB_SCHEMA"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_dba63e52","line":246,"updated":"2016-07-29 13:46:21.000000000","message":"if we really wanted to run the tests with the api database, we would need to return the api engine here and skip the migration below","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"7e430c80cb19030294fb781c108c88e34392dfa1","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        elif database \u003d\u003d \u0027api\u0027:"},{"line_number":244,"context_line":"            self.get_engine \u003d session.get_api_engine"},{"line_number":245,"context_line":"        elif database \u003d\u003d \u0027placement\u0027:"},{"line_number":246,"context_line":"            self.get_engine \u003d session.get_placement_engine"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _cache_schema(self):"},{"line_number":249,"context_line":"        global DB_SCHEMA"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_7ee8e0b4","line":246,"in_reply_to":"bacf61ea_9eab14d6","updated":"2016-07-29 13:57:32.000000000","message":"That makes sense to me. Let\u0027s give it a try then.","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2b0d3866cc41e2579aff3fea40edd12a869046a4","unresolved":false,"context_lines":[{"line_number":243,"context_line":"        elif database \u003d\u003d \u0027api\u0027:"},{"line_number":244,"context_line":"            self.get_engine \u003d session.get_api_engine"},{"line_number":245,"context_line":"        elif database \u003d\u003d \u0027placement\u0027:"},{"line_number":246,"context_line":"            self.get_engine \u003d session.get_placement_engine"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def _cache_schema(self):"},{"line_number":249,"context_line":"        global DB_SCHEMA"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_9eab14d6","line":246,"in_reply_to":"bacf61ea_dba63e52","updated":"2016-07-29 13:50:04.000000000","message":"In general it doesn\u0027t matter which database all the tests run against, as long as they run correctly.\n\nHowever it would be good to have at least one test that demonstrates that things work when choosing and not choosing to set the new configuration setting.","commit_id":"da348e4abe031d92acfe23bb8e3edd9a73dada96"}]}
