)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c63800aef03c6020841a7a66681e736e75e8dc56","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Object paths can have non-ascii characters. Device dicts will"},{"line_number":10,"context_line":"have unicode values. Forming a string using both will cause the"},{"line_number":11,"context_line":"object path to be coerced to UTF8, which currently causes a"},{"line_number":12,"context_line":"UnicodeDecodeError. This causes _get_response() to not return"},{"line_number":13,"context_line":"and the recosntructor hangs."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7ffa3b31_639654f3","line":11,"range":{"start_line":11,"start_character":29,"end_line":11,"end_character":33},"updated":"2017-04-18 18:34:30.000000000","message":"nit: unicode\n\nThe object path is already UTF8; the trouble is that python, in trying to coerce those UTF8 bytes to unicode, defaults to decoding as ASCII.","commit_id":"83750cf79c958810767f8a78e739df5d7e3f5345"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c63800aef03c6020841a7a66681e736e75e8dc56","unresolved":false,"context_lines":[{"line_number":15,"context_line":"The call to _full_path() is moved outside of _get_response()"},{"line_number":16,"context_line":"(where its result is used in the exception handler logging)"},{"line_number":17,"context_line":"so that _get_response() will always return even if _full_path()"},{"line_number":18,"context_line":"raises an exception."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Unit tests are refactored to split out a new class with those"},{"line_number":21,"context_line":"tests using an object name and the _full_path method, so that"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7ffa3b31_a350dc5e","line":18,"updated":"2017-04-18 18:34:30.000000000","message":"Thanks for calling this part out; I was kinda wondering why we did that while just looking at the code. Now if we regress and _full_path *does* raise an exception, we\u0027ll *never* call _get_response.","commit_id":"83750cf79c958810767f8a78e739df5d7e3f5345"}],"swift/common/utils.py":[{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0e0d1168a54e716482b1060a14a5b220678cb79f","unresolved":false,"context_lines":[{"line_number":3249,"context_line":"    \"\"\""},{"line_number":3250,"context_line":"    Get valid parts of utf-8 str from str, unicode and even invalid utf-8 str"},{"line_number":3251,"context_line":""},{"line_number":3252,"context_line":"    :param str_or_unicode: a string or an unicode which can be invalid utf-8"},{"line_number":3253,"context_line":"    :returns: valid utf8 unicode"},{"line_number":3254,"context_line":"    \"\"\""},{"line_number":3255,"context_line":"    if isinstance(str_or_unicode, six.text_type):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_c8bb600c","line":3252,"range":{"start_line":3252,"start_character":39,"end_line":3252,"end_character":49},"updated":"2017-04-14 04:45:53.000000000","message":"nit: \"a unicode\"","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":3253,"context_line":"    :returns: valid utf8 unicode"},{"line_number":3254,"context_line":"    \"\"\""},{"line_number":3255,"context_line":"    if isinstance(str_or_unicode, six.text_type):"},{"line_number":3256,"context_line":"        (str_or_unicode, _len) \u003d utf8_encoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3257,"context_line":"    (valid_utf8_str, _len) \u003d utf8_decoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3258,"context_line":"    return valid_utf8_str"},{"line_number":3259,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_393c23c7","line":3256,"updated":"2017-04-17 21:53:00.000000000","message":"...and we can\u0027t just return str_or_unicode at this point because... surrogate pairs, I guess? Should we call that out in the docstring? And include a test case for it -- something like http://paste.openstack.org/show/606924/?","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":3253,"context_line":"    :returns: valid utf8 unicode"},{"line_number":3254,"context_line":"    \"\"\""},{"line_number":3255,"context_line":"    if isinstance(str_or_unicode, six.text_type):"},{"line_number":3256,"context_line":"        (str_or_unicode, _len) \u003d utf8_encoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3257,"context_line":"    (valid_utf8_str, _len) \u003d utf8_decoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3258,"context_line":"    return valid_utf8_str"},{"line_number":3259,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_2d384b3b","line":3256,"in_reply_to":"7ffa3b31_393c23c7","updated":"2017-04-18 13:14:07.000000000","message":"I see your point, but I\u0027m also concerned that there may be some reason for the deliberate encode/decode with \u0027replace\u0027 for the original use case of get_valid_utf8_str.\n\nI\u0027ve cooled off creating this helper function - it is overkill for the problem in hand - we can expect valid utf8 bytes for the object path, it is trivial to test that it\u0027s not already unexpectedly unicode, so I plan to simpligy the patch by doing the utf8 decode inline in the reconstructor and abandon this helper method refactoring.\n\nsorry for the review burden!","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"0e0d1168a54e716482b1060a14a5b220678cb79f","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"    \"\"\""},{"line_number":3255,"context_line":"    if isinstance(str_or_unicode, six.text_type):"},{"line_number":3256,"context_line":"        (str_or_unicode, _len) \u003d utf8_encoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3257,"context_line":"    (valid_utf8_str, _len) \u003d utf8_decoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3258,"context_line":"    return valid_utf8_str"},{"line_number":3259,"context_line":""},{"line_number":3260,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_e8585c57","line":3257,"range":{"start_line":3257,"start_character":5,"end_line":3257,"end_character":19},"updated":"2017-04-14 04:45:53.000000000","message":"If I understand correctly, this variable would be more appropriate as valid_utf8_unicode.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":3254,"context_line":"    \"\"\""},{"line_number":3255,"context_line":"    if isinstance(str_or_unicode, six.text_type):"},{"line_number":3256,"context_line":"        (str_or_unicode, _len) \u003d utf8_encoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3257,"context_line":"    (valid_utf8_str, _len) \u003d utf8_decoder(str_or_unicode, \u0027replace\u0027)"},{"line_number":3258,"context_line":"    return valid_utf8_str"},{"line_number":3259,"context_line":""},{"line_number":3260,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_b9593d5b","line":3257,"range":{"start_line":3257,"start_character":5,"end_line":3257,"end_character":19},"in_reply_to":"7ffa3b31_e8585c57","updated":"2017-04-18 13:14:07.000000000","message":"Agree, but see above comment","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"}],"swift/obj/reconstructor.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"48e142d135c5d3f77de62084f7a3ce1f53a25d81","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                       :class:`~swift.common.storage_policy.BaseStoragePolicy`"},{"line_number":229,"context_line":"        :returns: response"},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":"        full_path \u003d _full_path(node, part, path, policy)"},{"line_number":232,"context_line":"        resp \u003d None"},{"line_number":233,"context_line":"        try:"},{"line_number":234,"context_line":"            with ConnectionTimeout(self.conn_timeout):"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_cf5b0552","side":"PARENT","line":231,"updated":"2017-04-03 18:56:25.000000000","message":"We could also avoid the signature change and just calculate this in the try block?\n\nThe point is just that when you give a method to an AsyncPile it has to return.","commit_id":"bc9a7075d3accf3160043e8366745a1dd895b08d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c5bd06b83ef9af3693a52b9ca2028e4a697379c6","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                       :class:`~swift.common.storage_policy.BaseStoragePolicy`"},{"line_number":229,"context_line":"        :returns: response"},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":"        full_path \u003d _full_path(node, part, path, policy)"},{"line_number":232,"context_line":"        resp \u003d None"},{"line_number":233,"context_line":"        try:"},{"line_number":234,"context_line":"            with ConnectionTimeout(self.conn_timeout):"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_4a99231d","side":"PARENT","line":231,"updated":"2017-04-03 18:54:07.000000000","message":"if this raises an exception then the GreenAsyncPile never places a response on its queue and the call to its next() function never returns so the reconstructor stalls.","commit_id":"bc9a7075d3accf3160043e8366745a1dd895b08d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b7b1c4d272c45cfbdb3448bcd2a4a6318e89c57e","unresolved":false,"context_lines":[{"line_number":228,"context_line":"                       :class:`~swift.common.storage_policy.BaseStoragePolicy`"},{"line_number":229,"context_line":"        :returns: response"},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":"        full_path \u003d _full_path(node, part, path, policy)"},{"line_number":232,"context_line":"        resp \u003d None"},{"line_number":233,"context_line":"        try:"},{"line_number":234,"context_line":"            with ConnectionTimeout(self.conn_timeout):"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_78515362","side":"PARENT","line":231,"in_reply_to":"dfeb2761_cf5b0552","updated":"2017-04-04 08:22:42.000000000","message":"I considered that, but full_path is used in the except block so I\u0027d need to wrap the call to _full_path in it\u0027s own try/except. IDK, that seemed OTT for something that we don\u0027t expect to ever raise an exception unless there is a regression? If it were a public method I\u0027d be more reluctant to change the signature.","commit_id":"bc9a7075d3accf3160043e8366745a1dd895b08d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"48e142d135c5d3f77de62084f7a3ce1f53a25d81","unresolved":false,"context_lines":[{"line_number":86,"context_line":"            \u0027replication_ip\u0027: node[\u0027replication_ip\u0027],"},{"line_number":87,"context_line":"            \u0027replication_port\u0027: node[\u0027replication_port\u0027],"},{"line_number":88,"context_line":"            \u0027device\u0027: node[\u0027device\u0027],"},{"line_number":89,"context_line":"            \u0027part\u0027: part, \u0027path\u0027: relative_path,"},{"line_number":90,"context_line":"            \u0027policy\u0027: policy,"},{"line_number":91,"context_line":"        }"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_8a5f2b0d","line":89,"updated":"2017-04-03 18:56:25.000000000","message":"why not just go to unicode here?\n\nDo we ever pass in a unicode name somehow?  Do we need the inverse of utils.get_valid_utf8_str?","commit_id":"b1126c6f00b718b35264bc6ea24233ff2a224d69"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b7b1c4d272c45cfbdb3448bcd2a4a6318e89c57e","unresolved":false,"context_lines":[{"line_number":86,"context_line":"            \u0027replication_ip\u0027: node[\u0027replication_ip\u0027],"},{"line_number":87,"context_line":"            \u0027replication_port\u0027: node[\u0027replication_port\u0027],"},{"line_number":88,"context_line":"            \u0027device\u0027: node[\u0027device\u0027],"},{"line_number":89,"context_line":"            \u0027part\u0027: part, \u0027path\u0027: relative_path,"},{"line_number":90,"context_line":"            \u0027policy\u0027: policy,"},{"line_number":91,"context_line":"        }"},{"line_number":92,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_783a7334","line":89,"in_reply_to":"dfeb2761_8a5f2b0d","updated":"2017-04-04 08:22:42.000000000","message":"that would work too. In the end I went for applying the decode at line 286 where it is really obvious what it is that we are assuming is utf8 (i.e. metadata[\u0027name\u0027]) vs making an assumption that whatever gets passed to this method as a relative_path is utf8.","commit_id":"b1126c6f00b718b35264bc6ea24233ff2a224d69"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"48e142d135c5d3f77de62084f7a3ce1f53a25d81","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        pile \u003d GreenAsyncPile(len(part_nodes))"},{"line_number":284,"context_line":"        # the proxy ensures all object paths are valid utf8 so it is safe to"},{"line_number":285,"context_line":"        # decode utf8 here"},{"line_number":286,"context_line":"        obj_path \u003d datafile_metadata[\u0027name\u0027].decode(\u0027utf8\u0027)"},{"line_number":287,"context_line":"        for _node in part_nodes:"},{"line_number":288,"context_line":"            full_get_path \u003d _full_path("},{"line_number":289,"context_line":"                _node, job[\u0027partition\u0027], obj_path, job[\u0027policy\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_cf20a5b3","line":286,"updated":"2017-04-03 18:56:25.000000000","message":"what about object metadata?","commit_id":"b1126c6f00b718b35264bc6ea24233ff2a224d69"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"b7b1c4d272c45cfbdb3448bcd2a4a6318e89c57e","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        pile \u003d GreenAsyncPile(len(part_nodes))"},{"line_number":284,"context_line":"        # the proxy ensures all object paths are valid utf8 so it is safe to"},{"line_number":285,"context_line":"        # decode utf8 here"},{"line_number":286,"context_line":"        obj_path \u003d datafile_metadata[\u0027name\u0027].decode(\u0027utf8\u0027)"},{"line_number":287,"context_line":"        for _node in part_nodes:"},{"line_number":288,"context_line":"            full_get_path \u003d _full_path("},{"line_number":289,"context_line":"                _node, job[\u0027partition\u0027], obj_path, job[\u0027policy\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_2c182c73","line":286,"in_reply_to":"dfeb2761_cf20a5b3","updated":"2017-04-04 08:22:42.000000000","message":"not sure what you are asking - this item of metadata is based on the object path - or do you mean is there any similar validation of other metadata items?\n\nhmmm, maybe there should be a check that the value is bytes before decoding, in case that ever changes","commit_id":"b1126c6f00b718b35264bc6ea24233ff2a224d69"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c5bd06b83ef9af3693a52b9ca2028e4a697379c6","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        # decode utf8 here"},{"line_number":286,"context_line":"        obj_path \u003d datafile_metadata[\u0027name\u0027].decode(\u0027utf8\u0027)"},{"line_number":287,"context_line":"        for _node in part_nodes:"},{"line_number":288,"context_line":"            full_get_path \u003d _full_path("},{"line_number":289,"context_line":"                _node, job[\u0027partition\u0027], obj_path, job[\u0027policy\u0027])"},{"line_number":290,"context_line":"            pile.spawn(self._get_response, _node, job[\u0027partition\u0027],"},{"line_number":291,"context_line":"                       obj_path, headers, full_get_path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfeb2761_0abc3b67","line":288,"updated":"2017-04-03 18:54:07.000000000","message":"I moved this call here so that if it does ever raise an exception it does so in the main thread - previously the exception in _full_path inside _get_response caused the GreenAsyncPile to never return from next()","commit_id":"b1126c6f00b718b35264bc6ea24233ff2a224d69"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            \u0027device\u0027: node[\u0027device\u0027],"},{"line_number":90,"context_line":"            \u0027part\u0027: part, \u0027path\u0027: get_valid_utf8_unicode(relative_path),"},{"line_number":91,"context_line":"            \u0027policy\u0027: policy,"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"class RebuildingECDiskFileStream(object):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_7c41790b","line":92,"updated":"2017-04-17 21:53:00.000000000","message":"Is there any other part of this that may have non-ascii bytes? node[\u0027device\u0027], maybe? Although I think as an operator you\u0027d just be asking for trouble at that point...","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a548bd615bbf7a1ff45840ede73f43cb92acbaf1","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            \u0027device\u0027: node[\u0027device\u0027],"},{"line_number":90,"context_line":"            \u0027part\u0027: part, \u0027path\u0027: get_valid_utf8_unicode(relative_path),"},{"line_number":91,"context_line":"            \u0027policy\u0027: policy,"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"class RebuildingECDiskFileStream(object):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_60076a84","line":92,"in_reply_to":"7ffa3b31_04d0cc1e","updated":"2017-04-18 17:45:43.000000000","message":"Maybe I was thinking of when we used to deserialize with simplejson? Meh; like I said, I don\u0027t really remember well.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            \u0027device\u0027: node[\u0027device\u0027],"},{"line_number":90,"context_line":"            \u0027part\u0027: part, \u0027path\u0027: get_valid_utf8_unicode(relative_path),"},{"line_number":91,"context_line":"            \u0027policy\u0027: policy,"},{"line_number":92,"context_line":"        }"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"class RebuildingECDiskFileStream(object):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_04d0cc1e","line":92,"in_reply_to":"7ffa3b31_7c41790b","updated":"2017-04-18 13:14:07.000000000","message":"nodes are (de)serialized (from)to rings by json with default encoding of utf8 assumed for strings. Any strings in the node dict will be valid utf8 unicode.\n\nHmmm, unless it\u0027s a really old ring that was serialized as a pickled RingData. But we wouldn\u0027t have such an old ring for an EC policy.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"}],"test/probe/test_reconstructor_rebuild.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        if not require_durable:"},{"line_number":119,"context_line":"            req_headers.update("},{"line_number":120,"context_line":"                {\u0027X-Backend-Fragment-Preferences\u0027: json.dumps([])})"},{"line_number":121,"context_line":"        # node dict has unicode values so utf8 decode our path parts too in"},{"line_number":122,"context_line":"        # case they have non-ascii characters"},{"line_number":123,"context_line":"        headers, data \u003d direct_client.direct_get_object("},{"line_number":124,"context_line":"            node, part, self.account.decode(\u0027utf8\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_7cabb988","line":121,"updated":"2017-04-17 21:53:00.000000000","message":"I thought I remembered seeing node dicts that sometimes had unicode and sometimes had bytes, but now I don\u0027t remember under what circumstances I saw bytes...","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        if not require_durable:"},{"line_number":119,"context_line":"            req_headers.update("},{"line_number":120,"context_line":"                {\u0027X-Backend-Fragment-Preferences\u0027: json.dumps([])})"},{"line_number":121,"context_line":"        # node dict has unicode values so utf8 decode our path parts too in"},{"line_number":122,"context_line":"        # case they have non-ascii characters"},{"line_number":123,"context_line":"        headers, data \u003d direct_client.direct_get_object("},{"line_number":124,"context_line":"            node, part, self.account.decode(\u0027utf8\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_247068dc","line":121,"in_reply_to":"7ffa3b31_7cabb988","updated":"2017-04-18 13:14:07.000000000","message":"AFAICT they should have unicode, and we even fake that in unit tests:\n\nhttps://github.com/openstack/swift/blob/e4972f5ac731f97b26891012107b4b8492e171fa/test/unit/__init__.py#L243-L244\n\noh, unless it is a really old ring that was just a pickled a RingData instance. Hmmm, now that could have bytes in the node dict. But I think for probe tests we can assume a recently built ring?","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c63800aef03c6020841a7a66681e736e75e8dc56","unresolved":false,"context_lines":[{"line_number":119,"context_line":"            req_headers.update("},{"line_number":120,"context_line":"                {\u0027X-Backend-Fragment-Preferences\u0027: json.dumps([])})"},{"line_number":121,"context_line":"        # node dict has unicode values so utf8 decode our path parts too in"},{"line_number":122,"context_line":"        # case they have non-ascii characters"},{"line_number":123,"context_line":"        headers, data \u003d direct_client.direct_get_object("},{"line_number":124,"context_line":"            node, part, self.account.decode(\u0027utf8\u0027),"},{"line_number":125,"context_line":"            self.container_name.decode(\u0027utf8\u0027),"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ffa3b31_6338949f","line":122,"updated":"2017-04-18 18:34:30.000000000","message":"Should we be doing this in direct_client instead of as a one-off to fix just this probe test? Something like\n\ndiff --git a/swift/common/direct_client.py b/swift/common/direct_client.py\nindex cfc25d6..c04bc97 100644\n--- a/swift/common/direct_client.py\n+++ b/swift/common/direct_client.py\n@@ -43,7 +43,7 @@ class DirectClientException(ClientException):\n         # host can be used to override the node ip and port reported in\n         # the exception\n         host \u003d host if host is not None else node\n-        full_path \u003d quote(\u0027/%s/%s%s\u0027 % (node[\u0027device\u0027], part, path))\n+        full_path \u003d quote(\u0027/%s/%s%s\u0027 % (node[\u0027device\u0027], part, path.decode(\u0027utf8\u0027)))\n         msg \u003d \u0027%s server %s:%s direct %s %r gave status %s\u0027 % (\n             stype, host[\u0027ip\u0027], host[\u0027port\u0027], method, full_path, resp.status)\n         headers \u003d HeaderKeyDict(resp.getheaders())\ndiff --git a/test/probe/test_reconstructor_rebuild.py b/test/probe/test_reconstructor_rebuild.py\nindex 24c5eb9..db66a68 100644\n--- a/test/probe/test_reconstructor_rebuild.py\n+++ b/test/probe/test_reconstructor_rebuild.py\n@@ -121,9 +121,9 @@ class TestReconstructorRebuild(ECProbeTest):\n         # node dict has unicode values so utf8 decode our path parts too in\n         # case they have non-ascii characters\n         headers, data \u003d direct_client.direct_get_object(\n-            node, part, self.account.decode(\u0027utf8\u0027),\n-            self.container_name.decode(\u0027utf8\u0027),\n-            self.object_name.decode(\u0027utf8\u0027), headers\u003dreq_headers,\n+            node, part, self.account,\n+            self.container_name,\n+            self.object_name, headers\u003dreq_headers,\n             resp_chunk_size\u003d64 * 2 ** 20)\n         hasher \u003d md5()\n         for chunk in data:\n\nseems to do it... or I could submit it as a follow-up.","commit_id":"83750cf79c958810767f8a78e739df5d7e3f5345"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":3373,"context_line":"        for before, after in subtests.items():"},{"line_number":3374,"context_line":"            self.assertEqual(utils.clean_content_type(before), after)"},{"line_number":3375,"context_line":""},{"line_number":3376,"context_line":"    def test_get_valid_utf8_str(self):"},{"line_number":3377,"context_line":"        for val, expected in ("},{"line_number":3378,"context_line":"                (\u0027abc\u0027, \u0027abc\u0027),"},{"line_number":3379,"context_line":"                (u\u0027abc\u0027, \u0027abc\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_795ceb7d","line":3376,"updated":"2017-04-17 21:53:00.000000000","message":"Good; all of this passes on master, too.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":3373,"context_line":"        for before, after in subtests.items():"},{"line_number":3374,"context_line":"            self.assertEqual(utils.clean_content_type(before), after)"},{"line_number":3375,"context_line":""},{"line_number":3376,"context_line":"    def test_get_valid_utf8_str(self):"},{"line_number":3377,"context_line":"        for val, expected in ("},{"line_number":3378,"context_line":"                (\u0027abc\u0027, \u0027abc\u0027),"},{"line_number":3379,"context_line":"                (u\u0027abc\u0027, \u0027abc\u0027),"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_b907fd1c","line":3376,"in_reply_to":"7ffa3b31_795ceb7d","updated":"2017-04-18 13:14:07.000000000","message":"Sorry for wasted review time but I\u0027ve decided this helper method and the extra tests is overkill","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":3379,"context_line":"                (u\u0027abc\u0027, \u0027abc\u0027),"},{"line_number":3380,"context_line":"                (u\u0027\\uc77c\\uc601\u0027, \u0027\\xec\\x9d\\xbc\\xec\\x98\\x81\u0027),"},{"line_number":3381,"context_line":"                (u\u0027\\uc77c\\uc601\u0027.encode(\u0027utf8\u0027), \u0027\\xec\\x9d\\xbc\\xec\\x98\\x81\u0027),"},{"line_number":3382,"context_line":"                (\u0027\\xec\\x9d\\xbc\\xec\\x98\u0027, \u0027\\xec\\x9d\\xbc\\xef\\xbf\\xbd\u0027)):"},{"line_number":3383,"context_line":"            actual \u003d utils.get_valid_utf8_str(val)"},{"line_number":3384,"context_line":"            self.assertEqual(expected, actual)"},{"line_number":3385,"context_line":"            self.assertIsInstance(actual, six.binary_type)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_d9c09f2d","line":3382,"range":{"start_line":3382,"start_character":30,"end_line":3382,"end_character":38},"updated":"2017-04-17 21:53:00.000000000","message":"Got it, so *this part* is invalid, and gets replaced by \\xef\\xbf\\xbd which is the utf-8 encoding of http://www.fileformat.info/info/unicode/char/fffd/index.htm","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"e4e535ef72d0d0c1cac2c0a56ecfe6f06a7283ab","unresolved":false,"context_lines":[{"line_number":3380,"context_line":"                (u\u0027\\uc77c\\uc601\u0027, \u0027\\xec\\x9d\\xbc\\xec\\x98\\x81\u0027),"},{"line_number":3381,"context_line":"                (u\u0027\\uc77c\\uc601\u0027.encode(\u0027utf8\u0027), \u0027\\xec\\x9d\\xbc\\xec\\x98\\x81\u0027),"},{"line_number":3382,"context_line":"                (\u0027\\xec\\x9d\\xbc\\xec\\x98\u0027, \u0027\\xec\\x9d\\xbc\\xef\\xbf\\xbd\u0027)):"},{"line_number":3383,"context_line":"            actual \u003d utils.get_valid_utf8_str(val)"},{"line_number":3384,"context_line":"            self.assertEqual(expected, actual)"},{"line_number":3385,"context_line":"            self.assertIsInstance(actual, six.binary_type)"},{"line_number":3386,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_3b7685da","line":3383,"updated":"2017-04-14 20:56:05.000000000","message":"perhaps not completely necessary given fixed expected strings, but you could add an explicit check for being valid utf8 str such as:\n            try:\n                actual.decode(\u0027utf-8\u0027)\n            except UnicodeError:\n                self.fail(\u0027not valid utf-8\u0027)\nThis would be similar to how the next test asserts actual is a six.text_type.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":12279,"name":"Janie Richling","email":"jrichli@us.ibm.com","username":"jrichli"},"change_message_id":"e4e535ef72d0d0c1cac2c0a56ecfe6f06a7283ab","unresolved":false,"context_lines":[{"line_number":3393,"context_line":"                (\u0027\\xec\\x9d\\xbc\\xec\\x98\u0027, u\u0027\\uc77c\\ufffd\u0027)):"},{"line_number":3394,"context_line":"            actual \u003d utils.get_valid_utf8_unicode(val)"},{"line_number":3395,"context_line":"            self.assertEqual(expected, actual)"},{"line_number":3396,"context_line":"            self.assertIsInstance(actual, six.text_type)"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"    def test_quote(self):"},{"line_number":3399,"context_line":"        res \u003d utils.quote(\u0027/v1/a/c3/subdirx/\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_9b3599fa","line":3396,"updated":"2017-04-14 20:56:05.000000000","message":"I am not a pro when it comes to py2 and py3 differences.  I see that six.text_type is unicode() in py2 and str in py3.  \nWill the function name \u0027get_valid_utf8_unicode\u0027 still be accurate when we switch to py3?  Similar question for \u0027get_valid_utf8_str\u0027.  If the naming needs to be changed to something that would make sense for both py2 and py3, then that might change more than just the naming of the functions - but variables and comments and such.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":3393,"context_line":"                (\u0027\\xec\\x9d\\xbc\\xec\\x98\u0027, u\u0027\\uc77c\\ufffd\u0027)):"},{"line_number":3394,"context_line":"            actual \u003d utils.get_valid_utf8_unicode(val)"},{"line_number":3395,"context_line":"            self.assertEqual(expected, actual)"},{"line_number":3396,"context_line":"            self.assertIsInstance(actual, six.text_type)"},{"line_number":3397,"context_line":""},{"line_number":3398,"context_line":"    def test_quote(self):"},{"line_number":3399,"context_line":"        res \u003d utils.quote(\u0027/v1/a/c3/subdirx/\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_59cfefaf","line":3396,"in_reply_to":"7ffa3b31_9b3599fa","updated":"2017-04-17 21:53:00.000000000","message":"IME with py2 and py3, I\u0027ve found:\n\n- \u0027unicode\u0027 is unambiguously unicode strings,\n- \u0027bytes\u0027 is unambiguously binary strings, and\n- \u0027str\u0027 generally means \"native string\".\n\nSo *maybe* it\u0027d be worth having a get_valid_utf8_bytes? Only I\u0027m willing to bet some third-party middlewares rely on get_valid_utf8_str, so we ought to keep that as an alias.\n\nWhich means we have two ways to do something... so we probably want to flag the old one as deprecated... and we *still* probably won\u0027t ever remove it...\n\nMeh. get_valid_utf8_str and get_valid_utf8_unicode are clear enough. *Might* still be worth littering the tests with b\u0027\u0027 to be explicit about when we want to be dealing with byte strings as opposed to native ones, but I\u0027d be fine punting on that until we actually bring this under py3 unit tests.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"}],"test/unit/obj/test_reconstructor.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":1204,"context_line":"        return next(self.ts_iter)"},{"line_number":1205,"context_line":""},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"@patch_policies(with_ec_default\u003dTrue)"},{"line_number":1208,"context_line":"class TestObjectReconstructor(BaseTestObjectReconstructor):"},{"line_number":1209,"context_line":"    def test_handoffs_only_default(self):"},{"line_number":1210,"context_line":"        # sanity neither option added to default conf"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_1c9ef553","line":1207,"updated":"2017-04-17 21:53:00.000000000","message":"We can\u0027t just put this on the base class and drop it from all the subclasses?","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":1204,"context_line":"        return next(self.ts_iter)"},{"line_number":1205,"context_line":""},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"@patch_policies(with_ec_default\u003dTrue)"},{"line_number":1208,"context_line":"class TestObjectReconstructor(BaseTestObjectReconstructor):"},{"line_number":1209,"context_line":"    def test_handoffs_only_default(self):"},{"line_number":1210,"context_line":"        # sanity neither option added to default conf"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_6d66532c","line":1207,"in_reply_to":"7ffa3b31_1c9ef553","updated":"2017-04-18 13:14:07.000000000","message":"Done","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c3850040f95ac31a92263f621d2de3edc8a11f6c","unresolved":false,"context_lines":[{"line_number":3367,"context_line":""},{"line_number":3368,"context_line":"        message_base \u003d \\"},{"line_number":3369,"context_line":"            \"Invalid resp from 10.0.0.0:1000/sda/0%s policy#0\" % \\"},{"line_number":3370,"context_line":"            self.obj_path.decode(\u0027utf-8\u0027)"},{"line_number":3371,"context_line":""},{"line_number":3372,"context_line":"        test_missing_header("},{"line_number":3373,"context_line":"            \u0027X-Object-Sysmeta-Ec-Frag-Index\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_7c9f3903","line":3370,"updated":"2017-04-17 21:53:00.000000000","message":"So our log line is now unicode? We\u0027re sure that won\u0027t break anything?","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a548bd615bbf7a1ff45840ede73f43cb92acbaf1","unresolved":false,"context_lines":[{"line_number":3367,"context_line":""},{"line_number":3368,"context_line":"        message_base \u003d \\"},{"line_number":3369,"context_line":"            \"Invalid resp from 10.0.0.0:1000/sda/0%s policy#0\" % \\"},{"line_number":3370,"context_line":"            self.obj_path.decode(\u0027utf-8\u0027)"},{"line_number":3371,"context_line":""},{"line_number":3372,"context_line":"        test_missing_header("},{"line_number":3373,"context_line":"            \u0027X-Object-Sysmeta-Ec-Frag-Index\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_60908a44","line":3370,"in_reply_to":"7ffa3b31_58791791","updated":"2017-04-18 17:45:43.000000000","message":"I thought that came up with https://review.openstack.org/#/c/323950/ ? I.e. as the root cause of https://bugs.launchpad.net/swift/+bug/1580678\n\nBut as you say, I suppose it\u0027s already an issue because of the node dict and json serialization/deserialization; we certainly haven\u0027t made anything *worse*.","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a4e4a887d8f27013eaa749a69458ba4ef352747f","unresolved":false,"context_lines":[{"line_number":3367,"context_line":""},{"line_number":3368,"context_line":"        message_base \u003d \\"},{"line_number":3369,"context_line":"            \"Invalid resp from 10.0.0.0:1000/sda/0%s policy#0\" % \\"},{"line_number":3370,"context_line":"            self.obj_path.decode(\u0027utf-8\u0027)"},{"line_number":3371,"context_line":""},{"line_number":3372,"context_line":"        test_missing_header("},{"line_number":3373,"context_line":"            \u0027X-Object-Sysmeta-Ec-Frag-Index\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7ffa3b31_58791791","line":3370,"in_reply_to":"7ffa3b31_7c9f3903","updated":"2017-04-18 13:14:07.000000000","message":"It was unicode before (due to node dict values being unicode) and I believe we have instances of unicode log lines in replicator too, possibly elsewhere, for same reason.\n\ndo you think that could be a problem?","commit_id":"ebb378e7c9b63d2df3f3adf1d4f3ba71e4635085"}]}