)]}'
{"oslo_messaging/rpc/client.py":[{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"35abc21283d0f8fcd1785054384c0f8a28b01c0d","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_07784ac3","line":193,"updated":"2015-06-23 07:46:41.000000000","message":"Does it mean that 0, b\u0027\u0027, u\u0027\u0027, [], (), etc. are valid versions too? What\u0027s the purpose of the \"if version\" test?","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"a4207d8ae2dae003ce702387077eab5656c77d6b","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_28d1e4cd","line":193,"in_reply_to":"fa32b979_05b73ef6","updated":"2015-06-29 22:44:35.000000000","message":"Done","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"5078a2a67337ec9404d2834cc0ed6fad125ae796","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_c72fc29f","line":193,"in_reply_to":"fa32b979_07784ac3","updated":"2015-06-23 07:49:00.000000000","message":"One of the unit tests passed None in as a version. Is that valid? What should we restrict a version to being? I\u0027m happy to write this in a more strict manner but I wasn\u0027t sure what the restrictions are.","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"19a4440ac6499314afcbb29886c05ab3d644a403","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_4af3b90a","line":193,"in_reply_to":"fa32b979_07784ac3","updated":"2015-06-23 07:59:57.000000000","message":"in your example, only b\u0027\u0027 would pass this check. the rest aren\u0027t str types","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"f878fd672c301e17c74b83bd578c33de13995998","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_05b73ef6","line":193,"in_reply_to":"fa32b979_baf72c58","updated":"2015-06-26 08:03:54.000000000","message":"\u003e But that accepts an empty string. Is that a valid version?\n\nAh yes, you should reject empty string. But I think that you should reject 0, u\u0027\u0027, [], (), etc. You should rewrite your check to accept None, self._marker and non-empty strings.","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":9107,"name":"Victor Stinner","email":"vstinner@redhat.com","username":"haypo"},"change_message_id":"23c14f534c10450a47a682149581bb428aa7ea29","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_eb772cd2","line":193,"in_reply_to":"fa32b979_c72fc29f","updated":"2015-06-23 10:06:29.000000000","message":"If you want to accept None, replace \"if version and ...\" with \"if version is None or ...\" (or \"if version is not None and ...\").\n\nMay None should be replaced with _marker, or the opposite. I don\u0027t know.","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"8d686636767e8aa337db0751038bfee91c12b7de","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                version\u003d_marker, server\u003d_marker, fanout\u003d_marker,"},{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version and (not isinstance(version, str)"},{"line_number":194,"context_line":"                        and version is not self._marker):"},{"line_number":195,"context_line":"            raise exceptions.MessagingException(\"Version must be a string.\")"},{"line_number":196,"context_line":"        return self._prepare(self,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_baf72c58","line":193,"in_reply_to":"fa32b979_eb772cd2","updated":"2015-06-23 11:41:07.000000000","message":"But that accepts an empty string. Is that a valid version?","commit_id":"49777fb53eaf3d99ae55cfb9c7eb583561285a78"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"978eda238779bed0a64b4687611de60b3f206bbb","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version is not None and version is not self._marker:"},{"line_number":194,"context_line":"            version \u003d str(version)"},{"line_number":195,"context_line":"            try:"},{"line_number":196,"context_line":"                utils.version_is_compatible(version, version)"},{"line_number":197,"context_line":"            except (IndexError, ValueError):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_ad0e43e3","line":194,"updated":"2015-07-07 07:59:40.000000000","message":"This allows to be lazy on the object type of version, I don\u0027t think we should enforce str here, the doc is clear about the expected type.\n\nOtherwise we have to do that in some other place too, like version_cap, or the server side, to have the whole public API handling any type like you do here.\n\nBut I wonder if this shouldn\u0027t go to utils.version_is_compatible() and just raise ValueError if one of the argument is unparsable and that it\u0027s.\n\nBecause if that occurs, this a bug in the consumer application not something that can be fixed by the deployer. Raising a MessagingException if more something that unexpected and if the application retry with same argument perhaps it will works. But in this case, if the application retries it will fail again and again, and the deployer can fix anything.\n\nAlso adding this directly to utils.version_is_compatible(), permits to have the same check in client and server side.\n\n\nOtherwise if we keep this, this block must move to _prepare() to avoid code duplication.","commit_id":"266c10c9131abe0a157c0723609d2dbc47633481"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e4c0b6560282c01164c0b4cda8c54a0262269b00","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version is not None and version is not self._marker:"},{"line_number":194,"context_line":"            version \u003d str(version)"},{"line_number":195,"context_line":"            try:"},{"line_number":196,"context_line":"                utils.version_is_compatible(version, version)"},{"line_number":197,"context_line":"            except (IndexError, ValueError):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_29ba110b","line":194,"in_reply_to":"ba3cc151_4eb19b8e","updated":"2015-07-07 14:32:35.000000000","message":"On the exception class, there are others that derive from this class like InvalidTarget that don\u0027t make sense to retry either.","commit_id":"266c10c9131abe0a157c0723609d2dbc47633481"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"f08f609abd38c62599612030733974092a16e677","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                timeout\u003d_marker, version_cap\u003d_marker, retry\u003d_marker):"},{"line_number":192,"context_line":"        \"\"\"Prepare a method invocation context. See RPCClient.prepare().\"\"\""},{"line_number":193,"context_line":"        if version is not None and version is not self._marker:"},{"line_number":194,"context_line":"            version \u003d str(version)"},{"line_number":195,"context_line":"            try:"},{"line_number":196,"context_line":"                utils.version_is_compatible(version, version)"},{"line_number":197,"context_line":"            except (IndexError, ValueError):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_4eb19b8e","line":194,"in_reply_to":"ba3cc151_ad0e43e3","updated":"2015-07-07 14:25:46.000000000","message":"This is using version_is_compatible...\n\nThe issue is that by the time it was called before, it was usually well after prepare was called so it was very late into execution (at least in the neutron server case). I want it to fail during setup.","commit_id":"266c10c9131abe0a157c0723609d2dbc47633481"}]}