)]}'
{"nova/conf/opts.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"ac262ee348b560c2d9f30bbe48411f0231083120","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        configs \u003d mod.list_opts()"},{"line_number":68,"context_line":"        if type(configs) is tuple:"},{"line_number":69,"context_line":"            config_options.append(configs)"},{"line_number":70,"context_line":"        elif type(configs) is list:"},{"line_number":71,"context_line":"            config_options.extend(configs)"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            msg \u003d \"The \u0027%s\u0027 function should return a tuple like \"\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_0310eb55","line":70,"updated":"2015-12-21 17:57:37.000000000","message":"This is much too fragile. If we are changing how opts get imported, we might as well provide a more flexible way of handling list_opts().","commit_id":"dffb369f9a93e2badd77cc627baaa43ec3ba4878"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"3ac60e39e9e5aae83ba4bf2133fe3c1d6481010d","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        configs \u003d mod.list_opts()"},{"line_number":68,"context_line":"        if type(configs) is tuple:"},{"line_number":69,"context_line":"            config_options.append(configs)"},{"line_number":70,"context_line":"        elif type(configs) is list:"},{"line_number":71,"context_line":"            config_options.extend(configs)"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            msg \u003d \"The \u0027%s\u0027 function should return a tuple like \"\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_64a43c4a","line":70,"in_reply_to":"3a7e1126_0310eb55","updated":"2015-12-22 09:46:15.000000000","message":"Do you mean line 70 in particular? Right now I don\u0027t see why it is fragile. That it raises an exception when a module doesn\u0027t implement \"list_opts\" in the expected way was a plus in my opinion. In the case when a (new) contributors add a new module to the package they get feedback about what\u0027s expected.\n\nThe main reason I started this patch was the possible merge conflicts when we will only have one \"opts.py\" and it has to import all other modules to call them. The same issue we have with the \"nova/conf/__init__.py\" where we solved this with the placeholder comments.","commit_id":"dffb369f9a93e2badd77cc627baaa43ec3ba4878"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"bdc857435e3f65b9ac449c1a1868a7a0685f79bf","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        configs \u003d mod.list_opts()"},{"line_number":68,"context_line":"        if type(configs) is tuple:"},{"line_number":69,"context_line":"            config_options.append(configs)"},{"line_number":70,"context_line":"        elif type(configs) is list:"},{"line_number":71,"context_line":"            config_options.extend(configs)"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            msg \u003d \"The \u0027%s\u0027 function should return a tuple like \"\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_c4780b92","line":70,"in_reply_to":"3a7e1126_64a43c4a","updated":"2016-01-04 15:47:22.000000000","message":"My concern is if a list returned by list_opts() is not properly formed, this function will happily add it to config_options anyway. IOW, simply depending on the type of an argument is fragile.\n\nLooking at things, what we are returning is a tuple in the form of (Label, [list of things]). IMO, this would be more cleanly handled with a dict. Things like the scheduler opts would simply return a dict with keys of \"DEFAULT\", \"metrics\", etc., while others would be a dict with a single key. We could provide a temporary shim to convert existing tuples to the dict, and remove that when the conversion is complete. How does that sound?","commit_id":"dffb369f9a93e2badd77cc627baaa43ec3ba4878"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"f62952f33984808621f9b3651153dec6df471653","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        configs \u003d mod.list_opts()"},{"line_number":68,"context_line":"        if type(configs) is tuple:"},{"line_number":69,"context_line":"            config_options.append(configs)"},{"line_number":70,"context_line":"        elif type(configs) is list:"},{"line_number":71,"context_line":"            config_options.extend(configs)"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            msg \u003d \"The \u0027%s\u0027 function should return a tuple like \"\\"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa69d971_ffa4b9a6","line":70,"in_reply_to":"fa69d971_c4780b92","updated":"2016-01-04 16:23:56.000000000","message":"I see now that the type checks are too weak (and could easily made stronger). I also agree that the dict you propose would be clearer. The temporary shim you talk about would be in this module here? I\u0027m asking because I\u0027d like to avoid that the existing reviews from other contributors have to be changed to reflect the change of the expected structure (from list|tuple to dict). To be clear, I understand you in a way that the \"list_opts\" method in the modules would look like this:\n\n    def list_opts():\n        return { \"DEFAULT\": [ my_opt ] }","commit_id":"dffb369f9a93e2badd77cc627baaa43ec3ba4878"}]}