)]}'
{"swift/obj/diskfile.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7e9019e22c0467236706cbd574bee6f6b237097d","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    \"\"\""},{"line_number":150,"context_line":"    from_dir \u003d dirname(corrupted_file_path)"},{"line_number":151,"context_line":"    to_dir \u003d join(device_path, \u0027quarantined\u0027,"},{"line_number":152,"context_line":"                  get_data_dir(extract_policy_index(corrupted_file_path)),"},{"line_number":153,"context_line":"                  basename(from_dir))"},{"line_number":154,"context_line":"    invalidate_hash(dirname(from_dir))"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ae8d1ca_f7904597","line":152,"updated":"2014-05-29 07:43:14.000000000","message":"This line ends up calling get_data_dir (or rather get_policy_string) twice, the first time not handling the error properly causing it to run a second time which then will error out. Also causing to hit the Policies singleton twice. \n\nIt looks like the extract_policy_index is a refactored out, and in other cases you want to just log a warning, but this doesn\u0027t look neat to me. \n\nMy suggestions are to raise the value error pushing it up to be dealt with, or maybe pass a warn flag if you want to ignore the error. \n\nI\u0027m quite new to the code base, so if I\u0027m missing something here, this is just my 2 cents.","commit_id":"dc9331627220a78ced45810d2d21ddca067a149d"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"db11b50d730c94ededa8449f6d5564a63c696e9a","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    \"\"\""},{"line_number":150,"context_line":"    from_dir \u003d dirname(corrupted_file_path)"},{"line_number":151,"context_line":"    to_dir \u003d join(device_path, \u0027quarantined\u0027,"},{"line_number":152,"context_line":"                  get_data_dir(extract_policy_index(corrupted_file_path)),"},{"line_number":153,"context_line":"                  basename(from_dir))"},{"line_number":154,"context_line":"    invalidate_hash(dirname(from_dir))"},{"line_number":155,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ae8d1ca_f52af37c","line":152,"in_reply_to":"3ae8d1ca_f7904597","updated":"2014-05-29 14:07:12.000000000","message":"Thanks for the detailed look!  This was on purpose and I admit that its a little hokey but I needed to pull the index out of the path and once I had that written it seemed like a good opportunity to have the function serve 2 purposes thus the change a bit later on to also use this function but simply to check to make sure the index valid and warn if not.  If others also feel this is too goofy I can rework it","commit_id":"dc9331627220a78ced45810d2d21ddca067a149d"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"7e9019e22c0467236706cbd574bee6f6b237097d","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    from_dir \u003d dirname(corrupted_file_path)"},{"line_number":151,"context_line":"    to_dir \u003d join(device_path, \u0027quarantined\u0027,"},{"line_number":152,"context_line":"                  get_data_dir(extract_policy_index(corrupted_file_path)),"},{"line_number":153,"context_line":"                  basename(from_dir))"},{"line_number":154,"context_line":"    invalidate_hash(dirname(from_dir))"},{"line_number":155,"context_line":"    try:"},{"line_number":156,"context_line":"        renamer(from_dir, to_dir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ae8d1ca_97ce21b3","line":153,"updated":"2014-05-29 07:43:14.000000000","message":"Also if there is an error here, do we want to deal with it, or let the code just die? Or is it more that the extract_policy_index will _always_ get a correct policy index, so shouldn\u0027t fail? \n\nAgain I am new, but would rather it be fault tolerant, otherwise why do the check at all.","commit_id":"dc9331627220a78ced45810d2d21ddca067a149d"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"db11b50d730c94ededa8449f6d5564a63c696e9a","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    from_dir \u003d dirname(corrupted_file_path)"},{"line_number":151,"context_line":"    to_dir \u003d join(device_path, \u0027quarantined\u0027,"},{"line_number":152,"context_line":"                  get_data_dir(extract_policy_index(corrupted_file_path)),"},{"line_number":153,"context_line":"                  basename(from_dir))"},{"line_number":154,"context_line":"    invalidate_hash(dirname(from_dir))"},{"line_number":155,"context_line":"    try:"},{"line_number":156,"context_line":"        renamer(from_dir, to_dir)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3ae8d1ca_f50913de","line":153,"in_reply_to":"3ae8d1ca_97ce21b3","updated":"2014-05-29 14:07:12.000000000","message":"So we only want to warn, if there is any error index 0 will be used which means the \"legacy\" dir will be used and \u0027/quarantined/objects\u0027 and if there\u0027s a logger we warn that something fishy is going on. We could decide to raise an error of course and I\u0027m open to that - we didn\u0027t do it in the audit generate location case however that would have hit in the event that there was extra stuff in the obj dir that we didn\u0027t recognize.  Not clear how a corrupted file path reported could have an invalid index so maybe that is a better idea.  Will hold off until I hear some more input","commit_id":"dc9331627220a78ced45810d2d21ddca067a149d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75d2451aa8e342953409bb64d1f4639fc2a46724","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    from_dir \u003d dirname(corrupted_file_path)"},{"line_number":145,"context_line":"    to_dir \u003d join(device_path, \u0027quarantined\u0027,"},{"line_number":146,"context_line":"                  get_data_dir(extract_policy_index(corrupted_file_path)),"},{"line_number":147,"context_line":"                  basename(from_dir))"},{"line_number":148,"context_line":"    invalidate_hash(dirname(from_dir))"},{"line_number":149,"context_line":"    try:"},{"line_number":150,"context_line":"        renamer(from_dir, to_dir)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_5d65ee43","line":147,"updated":"2014-05-30 05:06:10.000000000","message":"I think you need to catch that ValueError when you build the data_dir part - this function should fail no matter how badly corrupt the path is.","commit_id":"449063fa0b317d844caae11e97d0c16dff263ac1"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"4ca61358081a666bcd054ff4a5f3d02a9d63ecb0","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    from_dir \u003d dirname(corrupted_file_path)"},{"line_number":145,"context_line":"    to_dir \u003d join(device_path, \u0027quarantined\u0027,"},{"line_number":146,"context_line":"                  get_data_dir(extract_policy_index(corrupted_file_path)),"},{"line_number":147,"context_line":"                  basename(from_dir))"},{"line_number":148,"context_line":"    invalidate_hash(dirname(from_dir))"},{"line_number":149,"context_line":"    try:"},{"line_number":150,"context_line":"        renamer(from_dir, to_dir)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_2aad2832","line":147,"in_reply_to":"1ae5cdf2_5d65ee43","updated":"2014-05-30 14:48:46.000000000","message":"Done assuming you meant \"shot *not* fail\" :)","commit_id":"449063fa0b317d844caae11e97d0c16dff263ac1"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"75d2451aa8e342953409bb64d1f4639fc2a46724","unresolved":false,"context_lines":[{"line_number":1010,"context_line":"            self._container \u003d None"},{"line_number":1011,"context_line":"            self._obj \u003d None"},{"line_number":1012,"context_line":"            self._datadir \u003d None"},{"line_number":1013,"context_line":"        self._tmpdir \u003d join(device_path, get_tmp_dir(policy_idx))"},{"line_number":1014,"context_line":"        self._metadata \u003d None"},{"line_number":1015,"context_line":"        self._data_file \u003d None"},{"line_number":1016,"context_line":"        self._fp \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_9d177651","line":1013,"updated":"2014-05-30 05:06:10.000000000","message":"reverting this change should trigger a unittest failure","commit_id":"449063fa0b317d844caae11e97d0c16dff263ac1"},{"author":{"_account_id":7479,"name":"paul luse","email":"paul.e.luse@intel.com","username":"peluse"},"change_message_id":"4ca61358081a666bcd054ff4a5f3d02a9d63ecb0","unresolved":false,"context_lines":[{"line_number":1010,"context_line":"            self._container \u003d None"},{"line_number":1011,"context_line":"            self._obj \u003d None"},{"line_number":1012,"context_line":"            self._datadir \u003d None"},{"line_number":1013,"context_line":"        self._tmpdir \u003d join(device_path, get_tmp_dir(policy_idx))"},{"line_number":1014,"context_line":"        self._metadata \u003d None"},{"line_number":1015,"context_line":"        self._data_file \u003d None"},{"line_number":1016,"context_line":"        self._fp \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_8c4adc81","line":1013,"in_reply_to":"1ae5cdf2_9d177651","updated":"2014-05-30 14:48:46.000000000","message":"Done","commit_id":"449063fa0b317d844caae11e97d0c16dff263ac1"}]}