)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1fd9f0297b2b5c4229d555eb511d701fb2a230bf","unresolved":true,"context_lines":[{"line_number":23,"context_line":"done unconditionally."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I74de55107a80af13df348f2bce49415b08028746"},{"line_number":28,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9527cd73_044eeec2","line":26,"updated":"2021-08-23 16:56:38.000000000","message":"we shoudl proably refence the ooo bug\nhttps://bugs.launchpad.net/tripleo/+bug/1940555\n\nsince this is showing up in there ci\n\nmy guess as to why we dont see this with devstack is ooo are setting the connect string to\n\nconnection\u003dmysql+pymysql://nova:Jl6ip4yYTetmtrYWydWwJAcmI@192.168.24.3/nova?read_default_file\u003d/etc/my.cnf.d/tripleo.cnf\u0026read_default_group\u003dtripleo\n\n\n\nso they are passing read_default_file\u003d/etc/my.cnf.d/tripleo.cnf\nand read_default_group\u003dtripleo\n\nit was the slashes in read_default_file that were url encoded and breakign the db migration in the ooo job.","commit_id":"adf5440b1b51208a846873881755ae80d02eb7cf"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"dbc31f1d40714b638d840e7369153d41c690a416","unresolved":false,"context_lines":[{"line_number":23,"context_line":"done unconditionally."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I74de55107a80af13df348f2bce49415b08028746"},{"line_number":28,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"41993842_93c6bd65","line":26,"in_reply_to":"59490611_58686311","updated":"2021-08-23 18:58:31.000000000","message":"Done","commit_id":"adf5440b1b51208a846873881755ae80d02eb7cf"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"649915faf6e1ad7d25a5c813f089147e3ed009e9","unresolved":true,"context_lines":[{"line_number":23,"context_line":"done unconditionally."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: I74de55107a80af13df348f2bce49415b08028746"},{"line_number":28,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"59490611_58686311","line":26,"in_reply_to":"9527cd73_044eeec2","updated":"2021-08-23 17:23:46.000000000","message":"Agree this should mention Closes-Bug: #1940555","commit_id":"adf5440b1b51208a846873881755ae80d02eb7cf"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8d7f41060136b9d04d322646bd12f88a6feae174","unresolved":true,"context_lines":[{"line_number":9,"context_line":"As part of \u0027nova.db.migration\u0027, it is necessary to override the"},{"line_number":10,"context_line":"\u0027sqlalchemy.url\u0027 config option used by alembic. We were doing this but"},{"line_number":11,"context_line":"we weren\u0027t handling the fact that we could receive encoded URL strings."},{"line_number":12,"context_line":"This causes issues for the ConfigParser instead used under the hood, as"},{"line_number":13,"context_line":"noted in the alembic docs [1]:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  value – the value. Note that this value is passed to ConfigParser.set,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ef5d0fdb_c962d2c3","line":12,"range":{"start_line":12,"start_character":40,"end_line":12,"end_character":47},"updated":"2021-08-25 14:30:17.000000000","message":"This is supposed to be \"instance\" ?","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3db1f1e6b8604b5591e4861aa0c0a10ee9ce294","unresolved":true,"context_lines":[{"line_number":18,"context_line":"  symbol must therefore be escaped, e.g. %%. The given value may refer"},{"line_number":19,"context_line":"  to another value already in the file using the interpolation format."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Resolve the issue by escaping % with %%. The engine.url is always encoded"},{"line_number":22,"context_line":"therefore this can be done unconditionally."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"[1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"976b7e05_b6aaa1cd","line":21,"updated":"2021-08-26 10:07:05.000000000","message":"Can you wrap at \u003c\u003d 72 chars?","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"818df8fdfc0f4aa3b646be1d5b5708e8d513da1b","unresolved":true,"context_lines":[{"line_number":9,"context_line":"As part of \u0027nova.db.migration\u0027, it is necessary to override the"},{"line_number":10,"context_line":"\u0027sqlalchemy.url\u0027 config option used by alembic. We were doing this but"},{"line_number":11,"context_line":"we weren\u0027t handling the fact that we could receive encoded URL strings."},{"line_number":12,"context_line":"This causes issues for the ConfigParser instead used under the hood, as"},{"line_number":13,"context_line":"noted in the alembic docs [1]:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  value – the value. Note that this value is passed to ConfigParser.set,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"2d770153_8ef4c383","line":12,"range":{"start_line":12,"start_character":40,"end_line":12,"end_character":47},"updated":"2021-08-26 16:21:17.000000000","message":"\"instance\" right?","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd34b6d66c4434e9a731d120f2ef40a124e809e2","unresolved":true,"context_lines":[{"line_number":9,"context_line":"As part of \u0027nova.db.migration\u0027, it is necessary to override the"},{"line_number":10,"context_line":"\u0027sqlalchemy.url\u0027 config option used by alembic. We were doing this but"},{"line_number":11,"context_line":"we weren\u0027t handling the fact that we could receive encoded URL strings."},{"line_number":12,"context_line":"This causes issues for the ConfigParser instead used under the hood, as"},{"line_number":13,"context_line":"noted in the alembic docs [1]:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"  value – the value. Note that this value is passed to ConfigParser.set,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"dab3e8b4_91462171","line":12,"range":{"start_line":12,"start_character":40,"end_line":12,"end_character":47},"in_reply_to":"2d770153_8ef4c383","updated":"2021-08-26 17:06:00.000000000","message":"yes","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"}],"nova/db/migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b3081960b4e1deb3b6cec82d0ed04e3057b3ba01","unresolved":true,"context_lines":[{"line_number":138,"context_line":"    engine \u003d _get_engine(database, context\u003dcontext)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    repository \u003d _find_migrate_repo(database)"},{"line_number":141,"context_line":"    config \u003d _find_alembic_conf(database)"},{"line_number":142,"context_line":"    # discard the URL encoded in alembic.ini in favour of the URL configured"},{"line_number":143,"context_line":"    # for the engine, casting from \u0027sqlalchemy.engine.url.URL\u0027 to str in the"},{"line_number":144,"context_line":"    # process"}],"source_content_type":"text/x-python","patch_set":2,"id":"a96e20bb_a9dcd73a","line":141,"range":{"start_line":141,"start_character":4,"end_line":141,"end_character":10},"updated":"2021-08-24 14:32:18.000000000","message":"hum so this is actuly constucted by alembic so we cant replace it with a config parser instance that has interpolation disabled","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7744d659a2d551a571eac8b1a2402fdbea0335d7","unresolved":true,"context_lines":[{"line_number":142,"context_line":"    # discard the URL encoded in alembic.ini in favour of the URL configured"},{"line_number":143,"context_line":"    # for the engine, casting from \u0027sqlalchemy.engine.url.URL\u0027 to str in the"},{"line_number":144,"context_line":"    # process"},{"line_number":145,"context_line":"    url \u003d urllib.parse.unquote(str(engine.url))"},{"line_number":146,"context_line":"    config.set_main_option(\u0027sqlalchemy.url\u0027, url)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # if we\u0027re in a deployment where sqlalchemy-migrate is already present,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dbcd1d5c_45012833","line":145,"range":{"start_line":145,"start_character":35,"end_line":145,"end_character":45},"updated":"2021-08-23 19:28:59.000000000","message":"Note: apparently by definition the engine url is encoded:\n\nhttps://docs.sqlalchemy.org/en/14/core/engines.html#database-urls","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"a0d27d783b02a01db157dd937ab82639ab5f649a","unresolved":true,"context_lines":[{"line_number":142,"context_line":"    # discard the URL encoded in alembic.ini in favour of the URL configured"},{"line_number":143,"context_line":"    # for the engine, casting from \u0027sqlalchemy.engine.url.URL\u0027 to str in the"},{"line_number":144,"context_line":"    # process"},{"line_number":145,"context_line":"    url \u003d urllib.parse.unquote(str(engine.url))"},{"line_number":146,"context_line":"    config.set_main_option(\u0027sqlalchemy.url\u0027, url)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # if we\u0027re in a deployment where sqlalchemy-migrate is already present,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3b96aa0e_95ba4abc","line":145,"range":{"start_line":145,"start_character":35,"end_line":145,"end_character":45},"in_reply_to":"4a6cdda2_78a8c836","updated":"2021-08-25 14:41:17.000000000","message":"OK, I\u0027m late to the party. What I described is what Sean already pointed to in https://github.com/sqlalchemy/alembic/issues/700. So are we going to do anything in Nova code-wise, or are we going to expect that any URLs that we are passed have their %% escaped and doubled up?","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"8d7f41060136b9d04d322646bd12f88a6feae174","unresolved":true,"context_lines":[{"line_number":142,"context_line":"    # discard the URL encoded in alembic.ini in favour of the URL configured"},{"line_number":143,"context_line":"    # for the engine, casting from \u0027sqlalchemy.engine.url.URL\u0027 to str in the"},{"line_number":144,"context_line":"    # process"},{"line_number":145,"context_line":"    url \u003d urllib.parse.unquote(str(engine.url))"},{"line_number":146,"context_line":"    config.set_main_option(\u0027sqlalchemy.url\u0027, url)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # if we\u0027re in a deployment where sqlalchemy-migrate is already present,"}],"source_content_type":"text/x-python","patch_set":2,"id":"4a6cdda2_78a8c836","line":145,"range":{"start_line":145,"start_character":35,"end_line":145,"end_character":45},"in_reply_to":"da392291_9bd6a20e","updated":"2021-08-25 14:30:17.000000000","message":"So hold on.\n\nset_main_option() supports Python string interpolation [1], so literal \u0027%\u0027 have to be escaped by doubling them up \u0027%%\u0027.\n\n*On top of that*, if I\u0027m reading Mel\u0027s comment correctly, the URL string itself needs to be urlencoded [2].\n\nSo, if the interpolation happens first, all we need to do is double up the \u0027%%\u0027 in order to make sure that the urlencoded URL makes its way down to the engine correctly, no?\n\n[1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option\n[2] https://docs.sqlalchemy.org/en/14/core/engines.html#database-urls","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b3081960b4e1deb3b6cec82d0ed04e3057b3ba01","unresolved":true,"context_lines":[{"line_number":142,"context_line":"    # discard the URL encoded in alembic.ini in favour of the URL configured"},{"line_number":143,"context_line":"    # for the engine, casting from \u0027sqlalchemy.engine.url.URL\u0027 to str in the"},{"line_number":144,"context_line":"    # process"},{"line_number":145,"context_line":"    url \u003d urllib.parse.unquote(str(engine.url))"},{"line_number":146,"context_line":"    config.set_main_option(\u0027sqlalchemy.url\u0027, url)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    # if we\u0027re in a deployment where sqlalchemy-migrate is already present,"}],"source_content_type":"text/x-python","patch_set":2,"id":"da392291_9bd6a20e","line":145,"range":{"start_line":145,"start_character":35,"end_line":145,"end_character":45},"in_reply_to":"dbcd1d5c_45012833","updated":"2021-08-24 14:32:18.000000000","message":"i think we actully need to do the oppiccite here.\n\nif enginurl is encoded we need to decode it backinto a raw string and then yes we could double up % symbols it they are ligimatly presnt although im not sure why we would want to allow them to be present.\n\nwe do not support the use of interperalation on our config files as far as i am aware and i would generally not consider % to be a good partice in a password or user name.\n\nso if do double them up to workaround config parsers interpolation we should also issue a waring to the operator to advise them against using % in the url in the first place","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5b96cd02028e6bce6d9f4651ada13cbcf28cc7f1","unresolved":true,"context_lines":[{"line_number":72,"context_line":"        os.path.abspath(os.path.dirname(__file__)),"},{"line_number":73,"context_line":"        database, \u0027alembic.ini\u0027)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    if _ALEMBIC_CONF.get(database) is None:"},{"line_number":76,"context_line":"        config \u003d alembic_config.Config(path)"},{"line_number":77,"context_line":"        # we don\u0027t want to use the logger configuration from the file, which is"},{"line_number":78,"context_line":"        # only really intended for the CLI"}],"source_content_type":"text/x-python","patch_set":3,"id":"b8d45d98_a9d108fa","line":75,"range":{"start_line":75,"start_character":7,"end_line":75,"end_character":20},"updated":"2021-08-25 20:13:13.000000000","message":"Hm, shouldn\u0027t we probably be resetting this global in nova/test.py like we do with the others?","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"818df8fdfc0f4aa3b646be1d5b5708e8d513da1b","unresolved":true,"context_lines":[{"line_number":145,"context_line":"    # string using a mix of url encode styles for different parts of the url."},{"line_number":146,"context_line":"    # since we are updating the alembic config parser instance we need to"},{"line_number":147,"context_line":"    # escape \u0027%\u0027 to \u0027%%\u0027 to account for ConfigParser\u0027s string interpolation."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    url \u003d str(engine.url).replace(\u0027%\u0027, \u0027%%\u0027)"},{"line_number":150,"context_line":"    config.set_main_option(\u0027sqlalchemy.url\u0027, url)"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"553da572_0cd62ed3","line":148,"updated":"2021-08-26 16:21:17.000000000","message":"nit: do we need this empty line?","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd34b6d66c4434e9a731d120f2ef40a124e809e2","unresolved":true,"context_lines":[{"line_number":145,"context_line":"    # string using a mix of url encode styles for different parts of the url."},{"line_number":146,"context_line":"    # since we are updating the alembic config parser instance we need to"},{"line_number":147,"context_line":"    # escape \u0027%\u0027 to \u0027%%\u0027 to account for ConfigParser\u0027s string interpolation."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    url \u003d str(engine.url).replace(\u0027%\u0027, \u0027%%\u0027)"},{"line_number":150,"context_line":"    config.set_main_option(\u0027sqlalchemy.url\u0027, url)"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"6fcb427c_cb7eb608","line":148,"in_reply_to":"553da572_0cd62ed3","updated":"2021-08-26 17:06:00.000000000","message":"no","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"}],"nova/test.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"920d063aac9dac005bb43c7c2b8dc96f95f3bec0","unresolved":true,"context_lines":[{"line_number":286,"context_line":"        # make sure that the wsgi app is fully initialized for all testcase"},{"line_number":287,"context_line":"        # instead of only once initialized for test worker"},{"line_number":288,"context_line":"        wsgi_app.init_global_data.reset()"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _setup_cells(self):"},{"line_number":291,"context_line":"        \"\"\"Setup a normal cellsv2 environment."},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"e9900e2b_be50411d","line":289,"updated":"2021-08-26 17:00:11.000000000","message":"On IRC did you mention you found we do need to reset _MIGRATE_REPO and _ALEMBIC_CONF here? Something like:\n\nmigration._MIGRATE_REPO \u003d {}\nmigration._ALEMBIC_CONF \u003d {}\n\nNote this isn\u0027t directly related to this patch but I think it should have been added when _MIGRATE_REPO and _ALEMBIC_CONF were first introduced. Alternatively, it could be its own separate patch to add the resets of the global vars.\n\n[1] https://github.com/openstack/nova/blob/2a78626a85954997d35f5fe62c50de297e2ca92d/nova/db/migration.py#L39","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"aeb83a0f7d42b978c2b54f661c246654969ca2de","unresolved":true,"context_lines":[{"line_number":286,"context_line":"        # make sure that the wsgi app is fully initialized for all testcase"},{"line_number":287,"context_line":"        # instead of only once initialized for test worker"},{"line_number":288,"context_line":"        wsgi_app.init_global_data.reset()"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _setup_cells(self):"},{"line_number":291,"context_line":"        \"\"\"Setup a normal cellsv2 environment."},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"dcb9eea1_9ee27721","line":289,"in_reply_to":"0f7a6878_9ec8f5e4","updated":"2021-08-26 17:18:24.000000000","message":"OK, I guess we can wait and see. In the past, not resetting globals has resulted in various nondeterministic test failures in the gate. Either way it could be its own patch if/when gate failures are encountered.","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c3a80395da7a71aef2f516d3a7d52148226f9e4f","unresolved":false,"context_lines":[{"line_number":286,"context_line":"        # make sure that the wsgi app is fully initialized for all testcase"},{"line_number":287,"context_line":"        # instead of only once initialized for test worker"},{"line_number":288,"context_line":"        wsgi_app.init_global_data.reset()"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _setup_cells(self):"},{"line_number":291,"context_line":"        \"\"\"Setup a normal cellsv2 environment."},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"eb97d374_0c75a207","line":289,"in_reply_to":"d3e17c53_8e65bc5d","updated":"2021-08-27 11:31:16.000000000","message":"Done","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4696258f869fc66970a5cf171e6c8bca200a47ad","unresolved":true,"context_lines":[{"line_number":286,"context_line":"        # make sure that the wsgi app is fully initialized for all testcase"},{"line_number":287,"context_line":"        # instead of only once initialized for test worker"},{"line_number":288,"context_line":"        wsgi_app.init_global_data.reset()"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _setup_cells(self):"},{"line_number":291,"context_line":"        \"\"\"Setup a normal cellsv2 environment."},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"d3e17c53_8e65bc5d","line":289,"in_reply_to":"dcb9eea1_9ee27721","updated":"2021-08-27 08:36:45.000000000","message":"You could just remove the globals. I don\u0027t think they\u0027re really needed. I kept them when reworking this stuff since they were already there, but they do seem to be a good example of a premature optimization. We won\u0027t call \u0027db_sync\u0027 and \u0027db_version\u0027 repeatedly in the same (non-test) process so there\u0027s no reason to keep the caching around IMO. If we remove it, that should be a separate precursor patch","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c3bc76ca37b8275ef8d6bc2c94476752aba7c14d","unresolved":true,"context_lines":[{"line_number":286,"context_line":"        # make sure that the wsgi app is fully initialized for all testcase"},{"line_number":287,"context_line":"        # instead of only once initialized for test worker"},{"line_number":288,"context_line":"        wsgi_app.init_global_data.reset()"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    def _setup_cells(self):"},{"line_number":291,"context_line":"        \"\"\"Setup a normal cellsv2 environment."},{"line_number":292,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"0f7a6878_9ec8f5e4","line":289,"in_reply_to":"e9900e2b_be50411d","updated":"2021-08-26 17:07:59.000000000","message":"so https://review.opendev.org/c/openstack/nova/+/805663/7/nova/tests/unit/db/test_migration.py#38 was to mock out this global to avoid that but it did not seam to be requried.","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"}],"nova/tests/unit/db/test_migration.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"7744d659a2d551a571eac8b1a2402fdbea0335d7","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        mock_get_engine.assert_called_once_with(\u0027main\u0027, context\u003dNone)"},{"line_number":65,"context_line":"        mock_find_repo.assert_called_once_with(\u0027main\u0027)"},{"line_number":66,"context_line":"        mock_find_conf.assert_called_once_with(\u0027main\u0027)"},{"line_number":67,"context_line":"        mock_find_conf.return_value.set_main_option.assert_called_once_with("},{"line_number":68,"context_line":"            \u0027sqlalchemy.url\u0027,"},{"line_number":69,"context_line":"            \u0027mysql+pymysql://nova:pass@192.168.24.3/nova?\u0027  # ..."},{"line_number":70,"context_line":"            \u0027read_default_file\u003d/etc/my.cnf.d/nova.cnf\u0027  # ..."}],"source_content_type":"text/x-python","patch_set":2,"id":"340ebbe8_e250bcd4","line":67,"range":{"start_line":67,"start_character":36,"end_line":67,"end_character":75},"updated":"2021-08-23 19:28:59.000000000","message":"I\u0027m wondering, would it be possible to test db sync with a database (this test class derives from test.NoDBTestCase)? Mocking this part makes it so we can\u0027t test/verify that different url inputs won\u0027t raise ValueError when set_main_option is called.\n\nI\u0027m thinking about it because I was looking at the doc for set_main_option [1] and it says:\n\n\"Note that this value is passed to ConfigParser.set, which supports variable interpolation using pyformat (e.g. %(some_value)s). A raw percent sign not part of an interpolation symbol must therefore be escaped, e.g. %%. The given value may refer to another value already in the file using the interpolation format.\"\n\nIt seems like we would need to do more to handle the case of a password that has a raw percent sign in it. Or am I missing something?\n\n[1] https://alembic.sqlalchemy.org/en/latest/api/config.html#alembic.config.Config.set_main_option","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"39509d2db6eaf7d1dd0bcf047df87431205f3870","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        mock_get_engine.assert_called_once_with(\u0027main\u0027, context\u003dNone)"},{"line_number":65,"context_line":"        mock_find_repo.assert_called_once_with(\u0027main\u0027)"},{"line_number":66,"context_line":"        mock_find_conf.assert_called_once_with(\u0027main\u0027)"},{"line_number":67,"context_line":"        mock_find_conf.return_value.set_main_option.assert_called_once_with("},{"line_number":68,"context_line":"            \u0027sqlalchemy.url\u0027,"},{"line_number":69,"context_line":"            \u0027mysql+pymysql://nova:pass@192.168.24.3/nova?\u0027  # ..."},{"line_number":70,"context_line":"            \u0027read_default_file\u003d/etc/my.cnf.d/nova.cnf\u0027  # ..."}],"source_content_type":"text/x-python","patch_set":2,"id":"c916d69b_a5125d35","line":67,"range":{"start_line":67,"start_character":36,"end_line":67,"end_character":75},"in_reply_to":"340ebbe8_e250bcd4","updated":"2021-08-24 14:36:27.000000000","message":"maybe that would be better done in a sperate func test or dedicated unit tests that use the opertunistic test fixture ot run with mysql/postgress.\n\ni think this unit test is initally sufficent but we coudl have a set of dedicated test to assert supprot for db sync with different parmaters.\nthat does feel like more of a func test to me if we are using a db other then sqlite even then i would question if its really a unit test.","commit_id":"6efb2d300fa566a5cfcd256746858fe4a5f25845"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5b96cd02028e6bce6d9f4651ada13cbcf28cc7f1","unresolved":true,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def test_db_sync_with_percent_in_connection_stirng(self):"},{"line_number":32,"context_line":"        qargs \u003d (\u0027\u0026read_default_group\u003dsomething_with_a_percent_%\u0027)"},{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1296c4e9_7faca1b8","line":31,"range":{"start_line":31,"start_character":48,"end_line":31,"end_character":54},"updated":"2021-08-25 20:13:13.000000000","message":"string","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def test_db_sync_with_percent_in_connection_stirng(self):"},{"line_number":32,"context_line":"        qargs \u003d (\u0027\u0026read_default_group\u003dsomething_with_a_percent_%\u0027)"},{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"95e4dd7e_1fcd34d5","line":31,"range":{"start_line":31,"start_character":48,"end_line":31,"end_character":54},"in_reply_to":"1296c4e9_7faca1b8","updated":"2021-08-26 11:38:32.000000000","message":"Ack","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5b96cd02028e6bce6d9f4651ada13cbcf28cc7f1","unresolved":true,"context_lines":[{"line_number":29,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def test_db_sync_with_percent_in_connection_stirng(self):"},{"line_number":32,"context_line":"        qargs \u003d (\u0027\u0026read_default_group\u003dsomething_with_a_percent_%\u0027)"},{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":35,"context_line":"        self.useFixture(fixtures.Database())"}],"source_content_type":"text/x-python","patch_set":3,"id":"b7bc1950_5d77efa0","line":32,"updated":"2021-08-25 20:13:13.000000000","message":"What do you think about putting some url encoding codes in here too so that we\u0027re doing more than just asserting what we pass to set_main_option for that case (in the other test below)?","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":true,"context_lines":[{"line_number":29,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def test_db_sync_with_percent_in_connection_stirng(self):"},{"line_number":32,"context_line":"        qargs \u003d (\u0027\u0026read_default_group\u003dsomething_with_a_percent_%\u0027)"},{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":35,"context_line":"        self.useFixture(fixtures.Database())"}],"source_content_type":"text/x-python","patch_set":3,"id":"b27e6578_6ac81023","line":32,"in_reply_to":"b7bc1950_5d77efa0","updated":"2021-08-26 11:38:32.000000000","message":"i can add in some other symbols sure","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5b96cd02028e6bce6d9f4651ada13cbcf28cc7f1","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":35,"context_line":"        self.useFixture(fixtures.Database())"},{"line_number":36,"context_line":"        migration.db_sync()"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class TestDBSync(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"0ebc27a7_8a8e1933","line":36,"updated":"2021-08-25 20:13:13.000000000","message":"I\u0027m wondering if we can also pull the conf (by using migration._find_alembic_conf) to assert that the url matches what we expect.","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"f332f697f0af3eedb8e938e808f96db301e24804","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":35,"context_line":"        self.useFixture(fixtures.Database())"},{"line_number":36,"context_line":"        migration.db_sync()"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class TestDBSync(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"14fab9db_e00bdc77","line":36,"in_reply_to":"0ebc27a7_8a8e1933","updated":"2021-08-26 08:51:20.000000000","message":"Yup that would be nice, I get the following with pdb:\n\n    (Pdb) migration._find_alembic_conf().get_main_option(\u0027sqlalchemy.url\u0027)\n    \u0027sqlite:///:memory:?read_default_group\u003dsomething_with_a_percent_%25\u0027","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":34,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":35,"context_line":"        self.useFixture(fixtures.Database())"},{"line_number":36,"context_line":"        migration.db_sync()"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"class TestDBSync(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1caa799b_9a7971c3","line":36,"in_reply_to":"14fab9db_e00bdc77","updated":"2021-08-26 11:38:32.000000000","message":"ya i can get this i was just looking up how to create a  spy function for set_main_option but i guess i have the migration module so i can just call get_main_option instead.","commit_id":"51cb5cdeb5ed0a0d4663697411b966f3c5623fc8"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3db1f1e6b8604b5591e4861aa0c0a10ee9ce294","unresolved":true,"context_lines":[{"line_number":11,"context_line":"#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":"import urllib"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from alembic import command as alembic_api"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a6438e0_e656cc2a","line":14,"updated":"2021-08-26 10:07:05.000000000","message":"mock is a third party library so this shouldn\u0027t have moved. Also, newline before this please","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":true,"context_lines":[{"line_number":11,"context_line":"#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the"},{"line_number":12,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":13,"context_line":"#    under the License."},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":"import urllib"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from alembic import command as alembic_api"}],"source_content_type":"text/x-python","patch_set":4,"id":"63489252_385a3856","line":14,"in_reply_to":"9a6438e0_e656cc2a","updated":"2021-08-26 11:38:32.000000000","message":"i moved it because imports and from statement should be split.\nand yes i know hacking does not require this but it allows it an i alway split them when i see them mixed.\n\ni find the way hacking decide order ot be very very unintiuive and i waste for too much time on it already to also have ot figure out how it works when we mix imports and form statements.\n\nif i respin again  i can splity this form the standard lib features to reflect its a third party lib and add the new line","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3db1f1e6b8604b5591e4861aa0c0a10ee9ce294","unresolved":true,"context_lines":[{"line_number":28,"context_line":"from nova.tests import fixtures"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":34,"context_line":"        qargs \u003d (\u0027read_default_group\u003dsomething with/a+percent_%-and%20symbols!\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"0547f495_db3b64db","line":31,"updated":"2021-08-26 10:07:05.000000000","message":"Any reason you derived from NoDBTestCase instead of TestCase? You\u0027re configuring a Database fixture below so I suspect it should have been the latter.","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":true,"context_lines":[{"line_number":28,"context_line":"from nova.tests import fixtures"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":34,"context_line":"        qargs \u003d (\u0027read_default_group\u003dsomething with/a+percent_%-and%20symbols!\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1d0c33d0_fa1108ae","line":31,"in_reply_to":"0547f495_db3b64db","updated":"2021-08-26 11:38:32.000000000","message":"yes i need to set the config options with self.flags  before any of the db fixture configuration happens\nso i cannot derive for TestCase\n\nalso i only need to set up on db not both the api and cell db so this is also faster.","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3db1f1e6b8604b5591e4861aa0c0a10ee9ce294","unresolved":true,"context_lines":[{"line_number":31,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":34,"context_line":"        qargs \u003d (\u0027read_default_group\u003dsomething with/a+percent_%-and%20symbols!\u0027)"},{"line_number":35,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":36,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":37,"context_line":"        self.useFixture(fixtures.Database())"}],"source_content_type":"text/x-python","patch_set":4,"id":"22d7bdd9_f2993e05","line":34,"updated":"2021-08-26 10:07:05.000000000","message":"style nit: you don\u0027t need these brackets","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":true,"context_lines":[{"line_number":31,"context_line":"class TestDBURL(test.NoDBTestCase):"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":34,"context_line":"        qargs \u003d (\u0027read_default_group\u003dsomething with/a+percent_%-and%20symbols!\u0027)"},{"line_number":35,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":36,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"},{"line_number":37,"context_line":"        self.useFixture(fixtures.Database())"}],"source_content_type":"text/x-python","patch_set":4,"id":"460d766f_f0065550","line":34,"in_reply_to":"22d7bdd9_f2993e05","updated":"2021-08-26 11:38:32.000000000","message":"i dont currently in a previious iteration i did because it was split over multiple lines because of pep8 \nyou are right that this is now not needed.","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a3db1f1e6b8604b5591e4861aa0c0a10ee9ce294","unresolved":true,"context_lines":[{"line_number":38,"context_line":"        migration.db_sync()"},{"line_number":39,"context_line":"        actual \u003d migration._find_alembic_conf().get_main_option(\u0027sqlalchemy.url\u0027)"},{"line_number":40,"context_line":"        expected \u003d (\"sqlite:///:memory:?read_default_group\u003dsomething+with%2Fa\""},{"line_number":41,"context_line":"                   \"+percent_%25-and+symbols%21\")"},{"line_number":42,"context_line":"        self.assertEquals(expected, actual)"},{"line_number":43,"context_line":"        self.assertEquals("},{"line_number":44,"context_line":"            urllib.parse.unquote_plus(url),"}],"source_content_type":"text/x-python","patch_set":4,"id":"c040ff4b_c78c47c2","line":41,"updated":"2021-08-26 10:07:05.000000000","message":"style nit: the indentation on this line is off. Can you drop this down a line of simply noqa it?\n\n  expected \u003d (\n      \"sqlite:///...\")\n\nor \n\n  expected \u003d \"sqlite:///...\"  # noqa: E501","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c07cfde2eedf98b0e0362d83f1e8453cc710839","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        migration.db_sync()"},{"line_number":39,"context_line":"        actual \u003d migration._find_alembic_conf().get_main_option(\u0027sqlalchemy.url\u0027)"},{"line_number":40,"context_line":"        expected \u003d (\"sqlite:///:memory:?read_default_group\u003dsomething+with%2Fa\""},{"line_number":41,"context_line":"                   \"+percent_%25-and+symbols%21\")"},{"line_number":42,"context_line":"        self.assertEquals(expected, actual)"},{"line_number":43,"context_line":"        self.assertEquals("},{"line_number":44,"context_line":"            urllib.parse.unquote_plus(url),"}],"source_content_type":"text/x-python","patch_set":4,"id":"f16bcee2_48a10644","line":41,"in_reply_to":"c040ff4b_c78c47c2","updated":"2021-08-26 11:38:32.000000000","message":"Ack","commit_id":"78c26de75f05b4e4a779c0d26fed3460fe598380"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"818df8fdfc0f4aa3b646be1d5b5708e8d513da1b","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    # NOTE(sean-k-mooney): this mock is not needed currently but"},{"line_number":37,"context_line":"    # keeping it here just in case that changes in the future."},{"line_number":38,"context_line":"    @mock.patch.dict(migration._ALEMBIC_CONF, {}, clear\u003dTrue)"},{"line_number":39,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":40,"context_line":"        self.assertEqual({}, migration._ALEMBIC_CONF)"},{"line_number":41,"context_line":"        qargs \u003d \u0027read_default_group\u003ddata with/a+percent_%-and%20symbols!\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"c936fb6e_79dac828","line":38,"updated":"2021-08-26 16:21:17.000000000","message":"If this is not needed, why have it? Are we guarding against a future where we modify migration._ALEMBIC_CONF to not end up being the empty dict here?","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd34b6d66c4434e9a731d120f2ef40a124e809e2","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    # NOTE(sean-k-mooney): this mock is not needed currently but"},{"line_number":37,"context_line":"    # keeping it here just in case that changes in the future."},{"line_number":38,"context_line":"    @mock.patch.dict(migration._ALEMBIC_CONF, {}, clear\u003dTrue)"},{"line_number":39,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":40,"context_line":"        self.assertEqual({}, migration._ALEMBIC_CONF)"},{"line_number":41,"context_line":"        qargs \u003d \u0027read_default_group\u003ddata with/a+percent_%-and%20symbols!\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"eb773523_52b7e6dd","line":38,"in_reply_to":"c936fb6e_79dac828","updated":"2021-08-26 17:06:00.000000000","message":"melwitt ask if we neeed to reset this global in a previous revisions.\n\nit turns out that no we dont but at that point i had already mocked it so i kept it.\n\ni can remove it","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"aeb83a0f7d42b978c2b54f661c246654969ca2de","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    # NOTE(sean-k-mooney): this mock is not needed currently but"},{"line_number":37,"context_line":"    # keeping it here just in case that changes in the future."},{"line_number":38,"context_line":"    @mock.patch.dict(migration._ALEMBIC_CONF, {}, clear\u003dTrue)"},{"line_number":39,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":40,"context_line":"        self.assertEqual({}, migration._ALEMBIC_CONF)"},{"line_number":41,"context_line":"        qargs \u003d \u0027read_default_group\u003ddata with/a+percent_%-and%20symbols!\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"94735314_aa4e2aac","line":38,"in_reply_to":"eb773523_52b7e6dd","updated":"2021-08-26 17:18:24.000000000","message":"+1 to remove it, mocking them isn\u0027t how they should be reset anyway, it should be reset globally in nova/test.py like the other globals.","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"818df8fdfc0f4aa3b646be1d5b5708e8d513da1b","unresolved":true,"context_lines":[{"line_number":37,"context_line":"    # keeping it here just in case that changes in the future."},{"line_number":38,"context_line":"    @mock.patch.dict(migration._ALEMBIC_CONF, {}, clear\u003dTrue)"},{"line_number":39,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":40,"context_line":"        self.assertEqual({}, migration._ALEMBIC_CONF)"},{"line_number":41,"context_line":"        qargs \u003d \u0027read_default_group\u003ddata with/a+percent_%-and%20symbols!\u0027"},{"line_number":42,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":43,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"310e49a7_66d938a5","line":40,"updated":"2021-08-26 16:21:17.000000000","message":"Ditto, what\u0027s the value of this line?","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd34b6d66c4434e9a731d120f2ef40a124e809e2","unresolved":true,"context_lines":[{"line_number":37,"context_line":"    # keeping it here just in case that changes in the future."},{"line_number":38,"context_line":"    @mock.patch.dict(migration._ALEMBIC_CONF, {}, clear\u003dTrue)"},{"line_number":39,"context_line":"    def test_db_sync_with_special_symbols_in_connection_string(self):"},{"line_number":40,"context_line":"        self.assertEqual({}, migration._ALEMBIC_CONF)"},{"line_number":41,"context_line":"        qargs \u003d \u0027read_default_group\u003ddata with/a+percent_%-and%20symbols!\u0027"},{"line_number":42,"context_line":"        url \u003d f\"sqlite:///:memory:?{qargs}\""},{"line_number":43,"context_line":"        self.flags(connection\u003durl, group\u003d\u0027database\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"42a51402_267b749b","line":40,"in_reply_to":"310e49a7_66d938a5","updated":"2021-08-26 17:06:00.000000000","message":"i was testing that my mocking of the global worked nothing more.\n\ni guess i can remove this and the previous mock.","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"818df8fdfc0f4aa3b646be1d5b5708e8d513da1b","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        # test is run on its own. when other test run before this without"},{"line_number":48,"context_line":"        # this fixture the engine will be reused, since the engine.url"},{"line_number":49,"context_line":"        # is immutable it will never get updated once its created so reusing"},{"line_number":50,"context_line":"        # the engine instance would break this test."},{"line_number":51,"context_line":"        engine \u003d enginefacade.writer.get_engine()"},{"line_number":52,"context_line":"        self.useFixture("},{"line_number":53,"context_line":"            fixtures.MonkeyPatch("}],"source_content_type":"text/x-python","patch_set":7,"id":"57b29938_1a935b7a","line":50,"updated":"2021-08-26 16:21:17.000000000","message":"This comment isn\u0027t really correct anymore - you\u0027re not using the fixture because it doesn\u0027t support per-test engines, you\u0027re just mocking _get_engine() to return the engine that you\u0027ve created.","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd34b6d66c4434e9a731d120f2ef40a124e809e2","unresolved":true,"context_lines":[{"line_number":47,"context_line":"        # test is run on its own. when other test run before this without"},{"line_number":48,"context_line":"        # this fixture the engine will be reused, since the engine.url"},{"line_number":49,"context_line":"        # is immutable it will never get updated once its created so reusing"},{"line_number":50,"context_line":"        # the engine instance would break this test."},{"line_number":51,"context_line":"        engine \u003d enginefacade.writer.get_engine()"},{"line_number":52,"context_line":"        self.useFixture("},{"line_number":53,"context_line":"            fixtures.MonkeyPatch("}],"source_content_type":"text/x-python","patch_set":7,"id":"0708d7e4_e3231a87","line":50,"in_reply_to":"57b29938_1a935b7a","updated":"2021-08-26 17:06:00.000000000","message":"right ill rewrite this.","commit_id":"8bf6040f01ec9072cc20f2ece2aa201a18e04149"}]}