)]}'
{"nova/api/openstack/placement/deploy.py":[{"author":{"_account_id":14511,"name":"Maciej Szankin","email":"maciej+review@szankin.pl","username":"mszankin"},"change_message_id":"e229ffe6024c316c6bfeeb0f0d297eac4c4ec29a","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    # wrong type."},{"line_number":52,"context_line":"    # TODO(cdent): Don\u0027t use a private method!?"},{"line_number":53,"context_line":"    try:"},{"line_number":54,"context_line":"        conf._get_group(\u0027cors\u0027)"},{"line_number":55,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":56,"context_line":"            {}, **conf.cors)"},{"line_number":57,"context_line":"    except cfg.NoSuchGroupError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a77a97e_81fe6eac","line":54,"range":{"start_line":54,"start_character":8,"end_line":54,"end_character":31},"updated":"2016-11-22 17:06:47.000000000","message":"Maybe instead of priv method import nova.conf and try accessing the cors group from there?","commit_id":"57a10bf4f7cf87f5ae0d31ca882b169ffac90b06"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f1131eea6a317527aa4d22965cd2637ff2d16fb9","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    # wrong type."},{"line_number":52,"context_line":"    # TODO(cdent): Don\u0027t use a private method!?"},{"line_number":53,"context_line":"    try:"},{"line_number":54,"context_line":"        conf._get_group(\u0027cors\u0027)"},{"line_number":55,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":56,"context_line":"            {}, **conf.cors)"},{"line_number":57,"context_line":"    except cfg.NoSuchGroupError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_1da2774b","line":54,"in_reply_to":"5a74a57a_1283d4cd","updated":"2016-11-28 01:25:25.000000000","message":"Yeah, like Maciej, I\u0027m not a fan of using a private method of an oslo_cfg class. However, I also understand the desire to not use any global configuration objects.\n\nChris, if you just passed **conf.cors like you do below here, and caught AttributeError, would that be OK?","commit_id":"57a10bf4f7cf87f5ae0d31ca882b169ffac90b06"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2d25e5ab11a1c3c4c13d614061c1a6b72e6bae9c","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    # wrong type."},{"line_number":52,"context_line":"    # TODO(cdent): Don\u0027t use a private method!?"},{"line_number":53,"context_line":"    try:"},{"line_number":54,"context_line":"        conf._get_group(\u0027cors\u0027)"},{"line_number":55,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":56,"context_line":"            {}, **conf.cors)"},{"line_number":57,"context_line":"    except cfg.NoSuchGroupError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_dda55147","line":54,"in_reply_to":"5a74a57a_1da2774b","updated":"2016-11-28 10:41:12.000000000","message":"IJay,  had considered that but dismissed it as I didn\u0027t want to mask some other AttributeError that was happening in the call to factory. Looking at the code, however, it appears it will be fine, so I\u0027ll give it a try.","commit_id":"57a10bf4f7cf87f5ae0d31ca882b169ffac90b06"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"423ca20f57e27380f671b62bf8b6fd4568da944b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    # wrong type."},{"line_number":52,"context_line":"    # TODO(cdent): Don\u0027t use a private method!?"},{"line_number":53,"context_line":"    try:"},{"line_number":54,"context_line":"        conf._get_group(\u0027cors\u0027)"},{"line_number":55,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":56,"context_line":"            {}, **conf.cors)"},{"line_number":57,"context_line":"    except cfg.NoSuchGroupError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a74a57a_1283d4cd","line":54,"in_reply_to":"7a77a97e_81fe6eac","updated":"2016-11-23 10:36:11.000000000","message":"This deploy method intentionally does not use global conf, so we need to be able to inspect the provided conf object and _get_group was the only thing I could find that was reliable.","commit_id":"57a10bf4f7cf87f5ae0d31ca882b169ffac90b06"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"402b2cc1b124d9fdcbfc36f70f0ab8f9aac498b6","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # AttributeError."},{"line_number":51,"context_line":"    try:"},{"line_number":52,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":53,"context_line":"            {}, **conf.cors)"},{"line_number":54,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":55,"context_line":"        cors_middleware \u003d None"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_3636edb4","line":53,"range":{"start_line":53,"start_character":16,"end_line":53,"end_character":27},"updated":"2016-11-30 11:53:25.000000000","message":"Hmm, so I think this is a problem.\n\nIf you call CORS.set_defaults(), does that mean you can then double check conf.cors.allowed_hosts is not None?\n\nI am totally confused with how this is meant to work now :S","commit_id":"1dec58345fd19a40305d867613d9f38dd3acc11b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"259d708a9a8db3920a69e3e21a68481d22c45fba","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # AttributeError."},{"line_number":51,"context_line":"    try:"},{"line_number":52,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":53,"context_line":"            {}, **conf.cors)"},{"line_number":54,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":55,"context_line":"        cors_middleware \u003d None"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_3bd41646","line":53,"updated":"2016-11-30 11:28:58.000000000","message":"The docs say we should do this:\n\n  from oslo_middleware import cors\n  from oslo_config import cfg\n\n  app \u003d cors.CORS(your_wsgi_application, cfg.CONF)\n\nDoes that not work for us?","commit_id":"1dec58345fd19a40305d867613d9f38dd3acc11b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c0f0a88a0f464ec3a05e43f34680b63a83170063","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # AttributeError."},{"line_number":51,"context_line":"    try:"},{"line_number":52,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":53,"context_line":"            {}, **conf.cors)"},{"line_number":54,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":55,"context_line":"        cors_middleware \u003d None"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_bbfe6636","line":53,"in_reply_to":"5a74a57a_1b04bac4","updated":"2016-11-30 11:37:09.000000000","message":"Actually, what we need here is something like:\n\n  from oslo_middleware import cors\n  from oslo_config import cfg\n  cors_middleware \u003d lambda x: cors.CORS(x, cfg.CONF)","commit_id":"1dec58345fd19a40305d867613d9f38dd3acc11b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"076f44cc7033e70ede3c799c733bb44fed3f94fa","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # AttributeError."},{"line_number":51,"context_line":"    try:"},{"line_number":52,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":53,"context_line":"            {}, **conf.cors)"},{"line_number":54,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":55,"context_line":"        cors_middleware \u003d None"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_76d065ca","line":53,"range":{"start_line":53,"start_character":16,"end_line":53,"end_character":27},"in_reply_to":"5a74a57a_3636edb4","updated":"2016-11-30 11:55:47.000000000","message":"This may be on interest: https://github.com/openstack/nova/blob/4f91ed3a547965ed96a22520edcfb783e7936e95/nova/common/config.py#L23","commit_id":"1dec58345fd19a40305d867613d9f38dd3acc11b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8b1bab66b8fcaca9e0124bdf8981adf3a17ffe09","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # AttributeError."},{"line_number":51,"context_line":"    try:"},{"line_number":52,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":53,"context_line":"            {}, **conf.cors)"},{"line_number":54,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":55,"context_line":"        cors_middleware \u003d None"},{"line_number":56,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_1b04bac4","line":53,"in_reply_to":"5a74a57a_3bd41646","updated":"2016-11-30 11:29:33.000000000","message":"http://docs.openstack.org/developer/oslo.middleware/cors.html#configuration-for-oslo-config","commit_id":"1dec58345fd19a40305d867613d9f38dd3acc11b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"28a92624cd0d68251a5ed29d4a556863ec53e21c","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # required because they are not in a hierarchy and only one ISA"},{"line_number":51,"context_line":"    # AttributeError."},{"line_number":52,"context_line":"    try:"},{"line_number":53,"context_line":"        conf.register_opts(cors.CORS_OPTS, \u0027cors\u0027)"},{"line_number":54,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":55,"context_line":"            {}, **conf.cors)"},{"line_number":56,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_71b5cf7c","line":53,"updated":"2016-11-30 12:17:55.000000000","message":"Nit: I guess this could live ouside the try/except\n\nBut doesn\u0027t this mean cfg.NoSuchOptError, cfg.NoSuchGroupError are not longer ever thrown in here.","commit_id":"e591cf7e33f998d40907be0a5917bc057a71dc38"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1f82da7b9f75409ab49b0adfee9b61b62df6fac7","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # required because they are not in a hierarchy and only one ISA"},{"line_number":51,"context_line":"    # AttributeError."},{"line_number":52,"context_line":"    try:"},{"line_number":53,"context_line":"        conf.register_opts(cors.CORS_OPTS, \u0027cors\u0027)"},{"line_number":54,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":55,"context_line":"            {}, **conf.cors)"},{"line_number":56,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_8cb40ed2","line":53,"in_reply_to":"5a74a57a_71b5cf7c","updated":"2016-11-30 12:32:02.000000000","message":"No, confusingly it does not. Something being registered is not the same as the group existing.\n\n(will move the registration outside the try block)","commit_id":"e591cf7e33f998d40907be0a5917bc057a71dc38"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"28a92624cd0d68251a5ed29d4a556863ec53e21c","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    # AttributeError."},{"line_number":52,"context_line":"    try:"},{"line_number":53,"context_line":"        conf.register_opts(cors.CORS_OPTS, \u0027cors\u0027)"},{"line_number":54,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":55,"context_line":"            {}, **conf.cors)"},{"line_number":56,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":57,"context_line":"        cors_middleware \u003d None"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    context_middleware \u003d auth.PlacementKeystoneContext"},{"line_number":60,"context_line":"    req_id_middleware \u003d oslo_middleware.RequestId"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_71d06fca","line":57,"range":{"start_line":54,"start_character":0,"end_line":57,"end_character":30},"updated":"2016-11-30 12:17:55.000000000","message":"I guess we should be looking for TypeError here now?\n\nIs this worth a unit test?","commit_id":"e591cf7e33f998d40907be0a5917bc057a71dc38"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1f82da7b9f75409ab49b0adfee9b61b62df6fac7","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        cors_middleware \u003d oslo_middleware.CORS.factory("},{"line_number":55,"context_line":"            {}, **conf.cors)"},{"line_number":56,"context_line":"    except (cfg.NoSuchOptError, cfg.NoSuchGroupError):"},{"line_number":57,"context_line":"        cors_middleware \u003d None"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    context_middleware \u003d auth.PlacementKeystoneContext"},{"line_number":60,"context_line":"    req_id_middleware \u003d oslo_middleware.RequestId"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_cc91a654","line":57,"in_reply_to":"5a74a57a_71d06fca","updated":"2016-11-30 12:32:02.000000000","message":"No we want the TypeError to raise because the TypeError is the indication that though cors is configured, it is not configured correctly.\n\nI really struggle to come up with unit tests for this sort of stuff, I\u0027m happy to https://blog.leafe.com/pair-development/ though?","commit_id":"e591cf7e33f998d40907be0a5917bc057a71dc38"},{"author":{"_account_id":14511,"name":"Maciej Szankin","email":"maciej+review@szankin.pl","username":"mszankin"},"change_message_id":"44b86d7811f2b31b077c0da8c105ffeabf2a7b07","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    else:"},{"line_number":41,"context_line":"        # Do not provide global conf to middleware here."},{"line_number":42,"context_line":"        auth_middleware \u003d auth_token.filter_factory("},{"line_number":43,"context_line":"            {}, olso_config_project\u003dproject_name)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # Pass in our CORS config, if any, manually as that\u0027s a)"},{"line_number":46,"context_line":"    # explicit, b) makes testing more straightfoward, c) let\u0027s"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_573f7086","line":43,"range":{"start_line":43,"start_character":16,"end_line":43,"end_character":35},"updated":"2017-01-05 19:56:53.000000000","message":"A typo, as indicated in https://bugs.launchpad.net/nova/+bug/1645306 .\ncdent - would you like to fix it here or have it addressed in this bug report?\nWas wandering how it works with this typo... Maybe the tests are not covering it.","commit_id":"32367d6abfcd0060db78e8aaaafbdb31b42ada65"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"7ccbc8fda49752d65d375774f13c6dab7e11ba9d","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    else:"},{"line_number":41,"context_line":"        # Do not provide global conf to middleware here."},{"line_number":42,"context_line":"        auth_middleware \u003d auth_token.filter_factory("},{"line_number":43,"context_line":"            {}, olso_config_project\u003dproject_name)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    # Pass in our CORS config, if any, manually as that\u0027s a)"},{"line_number":46,"context_line":"    # explicit, b) makes testing more straightfoward, c) let\u0027s"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_427a5078","line":43,"in_reply_to":"ba5201f7_573f7086","updated":"2017-01-05 20:30:59.000000000","message":"The current tests don\u0027t cover this section because the auth strategy is always noauth2.\n\nFurther, because of the typo, a global conf is used, and it just so happens to have the right info in it, so it just works in a non-test situation, but mostly by accident.\n\nThe fix should happen in its own patch, but we should do it soon. I\u0027ll find you in IRC.","commit_id":"32367d6abfcd0060db78e8aaaafbdb31b42ada65"}],"nova/tests/functional/api/openstack/placement/fixtures.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"259d708a9a8db3920a69e3e21a68481d22c45fba","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.conf.set_override(\u0027connection\u0027, \"sqlite://\","},{"line_number":58,"context_line":"                               group\u003d\u0027placement_database\u0027)"},{"line_number":59,"context_line":"        # Turn on CORS"},{"line_number":60,"context_line":"        self.conf.register_opts(cors.CORS_OPTS, \u0027cors\u0027)"},{"line_number":61,"context_line":"        self.conf.set_override(\u0027allowed_origin\u0027, \u0027http://valid.example.com\u0027,"},{"line_number":62,"context_line":"                               group\u003d\u0027cors\u0027)"},{"line_number":63,"context_line":"        config.parse_args([], default_config_files\u003dNone, configure_db\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a74a57a_7b24ceae","line":60,"updated":"2016-11-30 11:28:58.000000000","message":"hmm, thats odd... who does this in production?","commit_id":"1dec58345fd19a40305d867613d9f38dd3acc11b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"28a92624cd0d68251a5ed29d4a556863ec53e21c","unresolved":false,"context_lines":[{"line_number":56,"context_line":"                               group\u003d\u0027api_database\u0027)"},{"line_number":57,"context_line":"        self.conf.set_override(\u0027connection\u0027, \"sqlite://\","},{"line_number":58,"context_line":"                               group\u003d\u0027placement_database\u0027)"},{"line_number":59,"context_line":"        # Turn on CORS"},{"line_number":60,"context_line":"        self.conf.register_opts(cors.CORS_OPTS, \u0027cors\u0027)"},{"line_number":61,"context_line":"        # NOTE(cdent): If we remove this override, then the cors"},{"line_number":62,"context_line":"        # group ends up not existing in the conf, so when deploy.py"},{"line_number":63,"context_line":"        # wants to load the CORS middleware, it will not."}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_5193734f","line":60,"range":{"start_line":59,"start_character":0,"end_line":60,"end_character":55},"updated":"2016-11-30 12:17:55.000000000","message":"Should we not drop that bit now? I guess this is done directly in the code now?","commit_id":"e591cf7e33f998d40907be0a5917bc057a71dc38"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"1f82da7b9f75409ab49b0adfee9b61b62df6fac7","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.conf.set_override(\u0027connection\u0027, \"sqlite://\","},{"line_number":58,"context_line":"                               group\u003d\u0027placement_database\u0027)"},{"line_number":59,"context_line":"        # Turn on CORS"},{"line_number":60,"context_line":"        self.conf.register_opts(cors.CORS_OPTS, \u0027cors\u0027)"},{"line_number":61,"context_line":"        # NOTE(cdent): If we remove this override, then the cors"},{"line_number":62,"context_line":"        # group ends up not existing in the conf, so when deploy.py"},{"line_number":63,"context_line":"        # wants to load the CORS middleware, it will not."}],"source_content_type":"text/x-python","patch_set":3,"id":"5a74a57a_0c00fea9","line":60,"in_reply_to":"5a74a57a_5193734f","updated":"2016-11-30 12:32:02.000000000","message":"apparently you can\u0027t call set_override without the register first, the tests die with an oh so informative \"math domain error\" without it.","commit_id":"e591cf7e33f998d40907be0a5917bc057a71dc38"}]}