)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":12,"context_line":"  databases."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"  The warning message includes the object path and the list of node"},{"line_number":15,"context_line":"  IDs that returned 404."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"  Closes-Bug: #1647840"},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"f3a1f6f4_203793c2","line":15,"updated":"2025-11-05 20:07:32.000000000","message":"node_to_string(node) actually, which is even better!  e.g.\n\n```\ntest DEBUG: Error code 404 is returned from remote server 127.0.0.1:37351/sda1\ntest DEBUG: Error code 404 is returned from remote server 127.0.0.1:37351/sda1\ntest DEBUG: Error code 404 is returned from remote server 127.0.0.1:37351/sda1\ntest DEBUG: Update failed for /a/c/o /mnt/tmp/tmptdqatw8w/devices/sda1/async_pending/a83/06fbf0b514e5199dfc4e00f42eb5ea83-1762354234.31787\ntest WARNING: Update failed because all primaries return 404 /a/c/o [\u0027127.0.0.2:1/sda1\u0027, \u0027127.0.0.2:1/sda1\u0027, \u0027127.0.0.2:1/sda1\u0027]\n```","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4ebac9f2_96875640","updated":"2025-10-20 04:40:18.000000000","message":"Has a linked bug, tests cover the change, solid commit message -- this is a great first contribution, thank you!\n\nI wouldn\u0027t mind getting Clay\u0027s opinion on it, too (since he wrote up the bug), but I saw a couple things that might make this *even better*:\n\n- drop some extra log info -- `path` will include `obj`\n- see if there\u0027s maybe a spot in our existing error handling that would kinda fit better.","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"0966c34ccd29a639f1938ea1a422e7fca8b36f95","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1e9a7344_cafa71cc","updated":"2025-10-20 13:22:27.000000000","message":"Thank you so much for your code review comments, they helped me understand much more of how Swift works 😄","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ad6b9d6c_85be18c1","updated":"2025-11-05 20:07:32.000000000","message":"I sort of feel like maybe having ANY warning for this condition (even if it\u0027s just some of the time) is better than nothing?  +2\n\nBut normally I wouldn\u0027t want to +A a change that would then need to have a bug opened for \"the object-updater 404 warning doesn\u0027t work if there was some old success before the node went offline\"\n\nIf someone else wanted to +A I think I could understand that - and if I notice it merge as is, I\u0027ll open the bug and polish up the fix:\n\n966216: dnm (sq?): log when all attempts 404 | https://review.opendev.org/c/openstack/swift/+/966216\n\nAlternatively, if you want - just squash any of that change you find useful into here and I will quickly re-review/merge this one and abandon that one!\n\nI think the spirit of this change is a welcome improvement, and the cleanup on the return signature specifically is a nice maintainability improvement orthogonal to the new logging that addresses the related bug.\n\nAside from the replication\u003dTrue (minor ergonomics improvement) my only reservation is the frequency/reliability of the warning line.  The spirit of the bug was \"we had stuck asyncs and the updater acted like everything is fine\" - I could imagine a situation where a change that logged on `failures \u003d [404]` while `success \u003d [1, 2]` could (temporarily) generate more log lines that we want during a container rebalance; but I think the fix for THAT would be to only log the warning if all remaining primaries 404 AND it\u0027s starting to get old - which treads dangerously close into the actual/complete fix territory:\n\n527296: Quarantine asyncs older then reclaim_age | https://review.opendev.org/c/openstack/swift/+/527296\n\n^ but that isn\u0027t ready to merge, and adding a warning so you can SEE the problem, is a good first step towards *fixing* the problem.\n\nso it\u0027s a judgement call on if we want a less noisy signal that maybe sometimes fails - or a more noisy signal that some times if a false positive.  I\u0027m waffling!!!\n\nUnrelated (but I\u0027ll call it out anyway) while reading some of the existing tests I accidentally tried to cleanup some the test helpers:\n\n966210: tests: make obj-updater pickle stub barely better | https://review.opendev.org/c/openstack/swift/+/966210\n \n^ any review/feedback there would be welcome as well as it might make some of the denser/DAMP parts of these tests a little easier to spell.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"21a5d508d8697c7d0c530fc5131ebd8b9d894033","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ba9fe2bc_29c611ed","updated":"2025-10-21 20:11:36.000000000","message":"Looking good to me! I still want Clay to take a look (I thought I added him as a reviewer already, but I guess not).","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d8ddc10bdc97f6a89983324cf1e824a5bb86b74f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c620040a_33ce18d3","in_reply_to":"6800568f_1eff3770","updated":"2025-11-19 15:11:44.000000000","message":"\u003e thank you for your review\n\nyou\u0027re so welcome!  thank you for working on swift.\n\n\u003e or if you rather wait for a third code reviewer\n\nI think we have all the bandwidth we need to merge a fix for lp bug #1647840 between the two of us.  Let\u0027s do it!\n\nI\u0027ve thought about it more and I\u0027d prefer to have a log message than not.  Between the two changes we have a test for all_404_and_no_success AND all_404_but_previous_success...\n\nWhat do you think about making the success case a warning?\n\n```\nif all(404):\n    if success:\n        # well it worked at some point\n        level \u003d log.warning\n    else:\n        level \u003d log.error\n    level(\u0027Unable to remove async update for AUTH_foo (X success \u0026 Y 404\u0027)\n```","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"bb8642e6fe10ceac751f74a6821e59500c3cfc10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6800568f_1eff3770","in_reply_to":"ad6b9d6c_85be18c1","updated":"2025-11-11 14:03:37.000000000","message":"First of all, thank you for your review, it was very detailed and helpful! Second, I don\u0027t mind applying any of the changes from your comments, I just want to know from you and @tburke@nvidia.com if I should apply the changes already or if you rather wait for a third code reviewer. I\u0027m good with both options!","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d8ddc10bdc97f6a89983324cf1e824a5bb86b74f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"eeeca33e_1dd311ec","updated":"2025-11-19 15:11:44.000000000","message":"dagnabit, i\u0027d meant to reply to this last week 😞\n\n^ probably proof that it\u0027d have been better to just merge it and push up a follow-up when I had it checked out, but I can\u0027t that today so I\u0027m going to hand the ball back to you.  Don\u0027t give up!","commit_id":"ee9a4d304bdf284b67f863405417425088a24652"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"af91fd48c0d32e6f447cd45cf323a6f43359a215","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"bfc622ed_beec7dcf","updated":"2026-01-02 15:49:38.000000000","message":"Hey all! First of all I want to start thanking you for the patience on how long it took for me to finish addressing the code review comments, Q3 was insane! I just submitted the changes addressing Clay\u0027s last comments. Thank you for all the kind explanations, they\u0027ve been really helpful to understand more about Swift since I still have lots to learn (which is exciting). Please feel free to request any changes and I\u0027ll try to address them faster than took me to address the last time, I swear 😄","commit_id":"be01c21c220b4f1f70b5ca65a90d4e25702f7036"}],"swift/obj/updater.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":true,"context_lines":[{"line_number":848,"context_line":"            path \u003d \u0027/%s/%s/%s\u0027 % (acct, cont, update[\u0027obj\u0027])"},{"line_number":849,"context_line":"            events \u003d [spawn(self.object_update,"},{"line_number":850,"context_line":"                            node, part, update[\u0027op\u0027], path, headers_out)"},{"line_number":851,"context_line":"                      for node in nodes if node[\u0027id\u0027] not in successes]"},{"line_number":852,"context_line":"            success \u003d True"},{"line_number":853,"context_line":"            new_successes \u003d rewrite_pickle \u003d False"},{"line_number":854,"context_line":"            redirect \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"524f563b_35e580a0","line":851,"range":{"start_line":851,"start_character":43,"end_line":851,"end_character":70},"updated":"2025-10-20 04:40:18.000000000","message":"So if we previously got *some* successes, we\u0027d still log about it if all attempts made *this round* failed with 404. I wonder what that tells us about the state of the system...\n\nThat we\u0027ve got some stale asyncs from a too-long-out-of-commission object drive, I guess?","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":848,"context_line":"            path \u003d \u0027/%s/%s/%s\u0027 % (acct, cont, update[\u0027obj\u0027])"},{"line_number":849,"context_line":"            events \u003d [spawn(self.object_update,"},{"line_number":850,"context_line":"                            node, part, update[\u0027op\u0027], path, headers_out)"},{"line_number":851,"context_line":"                      for node in nodes if node[\u0027id\u0027] not in successes]"},{"line_number":852,"context_line":"            success \u003d True"},{"line_number":853,"context_line":"            new_successes \u003d rewrite_pickle \u003d False"},{"line_number":854,"context_line":"            redirect \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"c4210722_2697a3e4","line":851,"range":{"start_line":851,"start_character":43,"end_line":851,"end_character":70},"in_reply_to":"3ffd2825_76637419","updated":"2025-11-05 20:07:32.000000000","message":"I think it would be antithetical to the bug report to get 404 from all *remaining* nodes, refuse to cleanup the async, and then NOT warn about it (because at some undetermined history a old node accepted the PUT).\n\nI think I agree w/ Tim and Andressa on what the behavior SHOULD be; but in my testing the all(404) should be sufficient on it\u0027s own.","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"0966c34ccd29a639f1938ea1a422e7fca8b36f95","unresolved":true,"context_lines":[{"line_number":848,"context_line":"            path \u003d \u0027/%s/%s/%s\u0027 % (acct, cont, update[\u0027obj\u0027])"},{"line_number":849,"context_line":"            events \u003d [spawn(self.object_update,"},{"line_number":850,"context_line":"                            node, part, update[\u0027op\u0027], path, headers_out)"},{"line_number":851,"context_line":"                      for node in nodes if node[\u0027id\u0027] not in successes]"},{"line_number":852,"context_line":"            success \u003d True"},{"line_number":853,"context_line":"            new_successes \u003d rewrite_pickle \u003d False"},{"line_number":854,"context_line":"            redirect \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c7ddfb1_eaf28473","line":851,"range":{"start_line":851,"start_character":43,"end_line":851,"end_character":70},"in_reply_to":"524f563b_35e580a0","updated":"2025-10-20 13:22:27.000000000","message":"That\u0027s a good observation! My opinion is we should only log this warning when NO nodes have succeeded. Does that make sense? Or would you prefer we log it anyway but make the message clearer by changing to something like \"remaining primaries\"?","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"21a5d508d8697c7d0c530fc5131ebd8b9d894033","unresolved":true,"context_lines":[{"line_number":848,"context_line":"            path \u003d \u0027/%s/%s/%s\u0027 % (acct, cont, update[\u0027obj\u0027])"},{"line_number":849,"context_line":"            events \u003d [spawn(self.object_update,"},{"line_number":850,"context_line":"                            node, part, update[\u0027op\u0027], path, headers_out)"},{"line_number":851,"context_line":"                      for node in nodes if node[\u0027id\u0027] not in successes]"},{"line_number":852,"context_line":"            success \u003d True"},{"line_number":853,"context_line":"            new_successes \u003d rewrite_pickle \u003d False"},{"line_number":854,"context_line":"            redirect \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"3ffd2825_76637419","line":851,"range":{"start_line":851,"start_character":43,"end_line":851,"end_character":70},"in_reply_to":"5c7ddfb1_eaf28473","updated":"2025-10-21 20:11:36.000000000","message":"\u003e My opinion is we should only log this warning when NO nodes have succeeded\n\n... this update round, or ever? I could see arguments both ways. I think \"this round\" (which is what you\u0027ve got) is perfectly defensible.\n\nPlus, the fact that we log which nodes responded 404 means we can tell the cases apart already, which is great!","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":true,"context_lines":[{"line_number":914,"context_line":""},{"line_number":915,"context_line":"            if failures and len(failures) \u003d\u003d len(events):"},{"line_number":916,"context_line":"                if all(status \u003d\u003d 404 for status, _ in failures):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9e4a625a_34f45258","line":917,"updated":"2025-10-20 04:40:18.000000000","message":"I wonder if this might also warrant some new stat. Especially if we see this cropping up from a bunch of the disks in a recently-recommissioned node, it seems like a sign that ops will need to go looking for dark data because they brought back some drives that were out for more than a reclaim-age.\n\nOTOH, maybe the rash of warning logs from the one node would provide a similar sort of signal. \\*shrug\\*","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"21a5d508d8697c7d0c530fc5131ebd8b9d894033","unresolved":true,"context_lines":[{"line_number":914,"context_line":""},{"line_number":915,"context_line":"            if failures and len(failures) \u003d\u003d len(events):"},{"line_number":916,"context_line":"                if all(status \u003d\u003d 404 for status, _ in failures):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"}],"source_content_type":"text/x-python","patch_set":2,"id":"c9d6662f_9722e810","line":917,"in_reply_to":"83c714ad_24046a94","updated":"2025-10-21 20:11:36.000000000","message":"Eh, warning\u0027s probably sufficient; it certainly covers what the bug was looking for. We can always add the stat later if needed/wanted.","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"0966c34ccd29a639f1938ea1a422e7fca8b36f95","unresolved":true,"context_lines":[{"line_number":914,"context_line":""},{"line_number":915,"context_line":"            if failures and len(failures) \u003d\u003d len(events):"},{"line_number":916,"context_line":"                if all(status \u003d\u003d 404 for status, _ in failures):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"}],"source_content_type":"text/x-python","patch_set":2,"id":"83c714ad_24046a94","line":917,"in_reply_to":"9e4a625a_34f45258","updated":"2025-10-20 13:22:27.000000000","message":"Thanks for explaining the use case! I understand now - if a disk comes back online after being down for a long time, there could be lots of async pendings for deleted containers, and a metric would make it easier to spot this pattern than grepping logs. I\u0027m happy to add a counter for this (like `self.stats.all_404s` and `self.logger.increment(\u0027all_404s\u0027)`). Would you like me to include that in this patch, or would the warning logs be sufficient for now?","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":true,"context_lines":[{"line_number":916,"context_line":"                if all(status \u003d\u003d 404 for status, _ in failures):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_id for status, node_id in failures]})"},{"line_number":922,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"83ccf7a0_b8a6d2e5","line":919,"range":{"start_line":919,"start_character":25,"end_line":919,"end_character":32},"updated":"2025-10-20 04:40:18.000000000","message":"Do we need `obj` when we have `path`?","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"21a5d508d8697c7d0c530fc5131ebd8b9d894033","unresolved":false,"context_lines":[{"line_number":916,"context_line":"                if all(status \u003d\u003d 404 for status, _ in failures):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_id for status, node_id in failures]})"},{"line_number":922,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"884efb7f_68ba6a5c","line":919,"range":{"start_line":919,"start_character":25,"end_line":919,"end_character":32},"in_reply_to":"2cc18cff_79106733","updated":"2025-10-21 20:11:36.000000000","message":"Done","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"0966c34ccd29a639f1938ea1a422e7fca8b36f95","unresolved":true,"context_lines":[{"line_number":916,"context_line":"                if all(status \u003d\u003d 404 for status, _ in failures):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_id for status, node_id in failures]})"},{"line_number":922,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2cc18cff_79106733","line":919,"range":{"start_line":919,"start_character":25,"end_line":919,"end_character":32},"in_reply_to":"83ccf7a0_b8a6d2e5","updated":"2025-10-20 13:22:27.000000000","message":"Yeah I don\u0027t think we need it, I\u0027ll remove it","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":true,"context_lines":[{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_id for status, node_id in failures]})"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"            return rewrite_pickle, redirect"},{"line_number":924,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5db49273_8b099b80","line":921,"updated":"2025-10-20 04:40:18.000000000","message":"Should we slide this up into the previous `else:` block? Might even be able to do an `elif` to avoid the length-check:\n```\nif new_successes:\n    ...\nelif all(status \u003d\u003d 404 for status, _ in failures):\n    self.logger.warning(...)\n```","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"0966c34ccd29a639f1938ea1a422e7fca8b36f95","unresolved":true,"context_lines":[{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_id for status, node_id in failures]})"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"            return rewrite_pickle, redirect"},{"line_number":924,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d571defd_0566d22a","line":921,"in_reply_to":"5db49273_8b099b80","updated":"2025-10-20 13:22:27.000000000","message":"Good suggestion, I\u0027ll work on it","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"21a5d508d8697c7d0c530fc5131ebd8b9d894033","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(obj)s %(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027obj\u0027: update[\u0027obj\u0027], \u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_id for status, node_id in failures]})"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"            return rewrite_pickle, redirect"},{"line_number":924,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"14b89c12_5867a374","line":921,"in_reply_to":"d571defd_0566d22a","updated":"2025-10-21 20:11:36.000000000","message":"Done","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":true,"context_lines":[{"line_number":973,"context_line":"                    \u0027server %(node)s\u0027,"},{"line_number":974,"context_line":"                    {\u0027status\u0027: resp.status,"},{"line_number":975,"context_line":"                     \u0027node\u0027: node_to_string(node, replication\u003dTrue)})"},{"line_number":976,"context_line":"            return status, node[\u0027id\u0027], redirect"},{"line_number":977,"context_line":"        except Exception:"},{"line_number":978,"context_line":"            self.logger.exception(\u0027ERROR with remote server %s\u0027,"},{"line_number":979,"context_line":"                                  node_to_string(node, replication\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":2,"id":"4a40dad5_9db09f6d","line":976,"range":{"start_line":976,"start_character":27,"end_line":976,"end_character":37},"updated":"2025-10-20 04:40:18.000000000","message":"I wonder if we might want to start returning the whole `node` dict from here; I feel like ops would find the `node_to_string()` version more useful in logs than the `node_id`.\n\nWDYT? You took up the bug, which would *you* find more useful?\n\nIt\u0027s also OK to say, \"yeah, it\u0027d be nice, but it\u0027d require **so much** test churn\" -- I haven\u0027t actually looked at how feasible it is to tweak this return, but your `is_success(status) -\u003e status` change gave me hope. 😉","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"0966c34ccd29a639f1938ea1a422e7fca8b36f95","unresolved":true,"context_lines":[{"line_number":973,"context_line":"                    \u0027server %(node)s\u0027,"},{"line_number":974,"context_line":"                    {\u0027status\u0027: resp.status,"},{"line_number":975,"context_line":"                     \u0027node\u0027: node_to_string(node, replication\u003dTrue)})"},{"line_number":976,"context_line":"            return status, node[\u0027id\u0027], redirect"},{"line_number":977,"context_line":"        except Exception:"},{"line_number":978,"context_line":"            self.logger.exception(\u0027ERROR with remote server %s\u0027,"},{"line_number":979,"context_line":"                                  node_to_string(node, replication\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":2,"id":"f28d7bc6_7ba709b9","line":976,"range":{"start_line":976,"start_character":27,"end_line":976,"end_character":37},"in_reply_to":"4a40dad5_9db09f6d","updated":"2025-10-20 13:22:27.000000000","message":"Hey! First thing, thank you for your code review comments! I think this is a great idea actually, I will update the code","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":false,"context_lines":[{"line_number":856,"context_line":"            redirects \u003d set()"},{"line_number":857,"context_line":"            for event in events:"},{"line_number":858,"context_line":"                event_success, node_id, redirect \u003d event.wait()"},{"line_number":859,"context_line":"                if event_success is True:"},{"line_number":860,"context_line":"                    successes.append(node_id)"},{"line_number":861,"context_line":"                    new_successes \u003d True"},{"line_number":862,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f77eb4a_743a2923","side":"PARENT","line":859,"updated":"2025-11-05 20:07:32.000000000","message":"that... is... wild\n\nsomehow it looks slightly more sensible in context:\n\n75941: Multithread optimization for object updater | https://review.opendev.org/c/openstack/swift/+/75941\n\nprobably b/c back when we added the greenthreads we still thought 404 was success.","commit_id":"9232350af06336760de6323b50fd6765b322e81f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":false,"context_lines":[{"line_number":982,"context_line":"            elapsed \u003d time.time() - start"},{"line_number":983,"context_line":"            self.logger.timing(\u0027updater.timing.status.%s\u0027 % status,"},{"line_number":984,"context_line":"                               elapsed * 1000)"},{"line_number":985,"context_line":"        return HTTP_INTERNAL_SERVER_ERROR, node[\u0027id\u0027], redirect"},{"line_number":986,"context_line":""},{"line_number":987,"context_line":""},{"line_number":988,"context_line":"def main():"}],"source_content_type":"text/x-python","patch_set":3,"id":"47368750_d29f51c4","side":"PARENT","line":985,"updated":"2025-11-05 20:07:32.000000000","message":"WTF?  this method already returned status sometimes?  503 is a truth-y value!?  What\u0027s going on?","commit_id":"9232350af06336760de6323b50fd6765b322e81f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":855,"context_line":"            redirects \u003d set()"},{"line_number":856,"context_line":"            failures \u003d []"},{"line_number":857,"context_line":"            for node, event in events:"},{"line_number":858,"context_line":"                status, node_id, redirect \u003d event.wait()"},{"line_number":859,"context_line":"                if is_success(status):"},{"line_number":860,"context_line":"                    successes.append(node_id)"},{"line_number":861,"context_line":"                    new_successes \u003d True"}],"source_content_type":"text/x-python","patch_set":3,"id":"18bb6178_07194467","line":858,"updated":"2025-11-05 20:07:32.000000000","message":"something about\n```\nfor node, e in events:\n    _, node_id, _ \u003d e()\n```\n\nlooks weird, like is `node_id \u003d\u003d node[\u0027id\u0027]` ever NOT true!?","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"21a5d508d8697c7d0c530fc5131ebd8b9d894033","unresolved":true,"context_lines":[{"line_number":911,"context_line":"                if new_successes:"},{"line_number":912,"context_line":"                    update[\u0027successes\u0027] \u003d successes"},{"line_number":913,"context_line":"                    rewrite_pickle \u003d True"},{"line_number":914,"context_line":"                elif (not successes and"},{"line_number":915,"context_line":"                      all(status \u003d\u003d 404"},{"line_number":916,"context_line":"                          for status, _ in failures)):"},{"line_number":917,"context_line":"                    self.logger.warning("}],"source_content_type":"text/x-python","patch_set":3,"id":"43287a81_84732e54","line":914,"range":{"start_line":914,"start_character":22,"end_line":914,"end_character":39},"updated":"2025-10-21 20:11:36.000000000","message":"nit: Can drop this; we already know `not success` from L868","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":38496,"name":"Andressa Cabistani","display_name":"Andressa","email":"acabistani@gmail.com","username":"andressadotpy"},"change_message_id":"12cca733ec8e74c86d02f184b807692c846910ea","unresolved":true,"context_lines":[{"line_number":911,"context_line":"                if new_successes:"},{"line_number":912,"context_line":"                    update[\u0027successes\u0027] \u003d successes"},{"line_number":913,"context_line":"                    rewrite_pickle \u003d True"},{"line_number":914,"context_line":"                elif (not successes and"},{"line_number":915,"context_line":"                      all(status \u003d\u003d 404"},{"line_number":916,"context_line":"                          for status, _ in failures)):"},{"line_number":917,"context_line":"                    self.logger.warning("}],"source_content_type":"text/x-python","patch_set":3,"id":"cb33aff3_22f1c17d","line":914,"range":{"start_line":914,"start_character":22,"end_line":914,"end_character":39},"in_reply_to":"43287a81_84732e54","updated":"2025-10-23 00:29:57.000000000","message":"Hey! I want to clarify what I implemented because maybe I am missing something. The code in this line is checking `not successes` a list initialized in the line 839, so when doing `not successes` the code is actually checking if it not once had a success stored in that list, because if at least one success happened, the message won\u0027t log. I added a unit test to check that the scenario [201, 404, 404] won\u0027t log any message. Before your first round of reviews, I was logging every time there was at least one failure. Does that makes sense or am I missing something?","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":911,"context_line":"                if new_successes:"},{"line_number":912,"context_line":"                    update[\u0027successes\u0027] \u003d successes"},{"line_number":913,"context_line":"                    rewrite_pickle \u003d True"},{"line_number":914,"context_line":"                elif (not successes and"},{"line_number":915,"context_line":"                      all(status \u003d\u003d 404"},{"line_number":916,"context_line":"                          for status, _ in failures)):"},{"line_number":917,"context_line":"                    self.logger.warning("}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae1ed69_bcedc0ab","line":914,"range":{"start_line":914,"start_character":22,"end_line":914,"end_character":39},"in_reply_to":"4724cb1b_eca605d0","updated":"2025-11-05 20:07:32.000000000","message":"wait, this is confusing...\n\nsuccesses \u003d update.get(\u0027successes\u0027, [])\n\nso this is explicitly NOT logging the warning EVER if ANY node previously succeeded?\n\n```\n-                elif (not successes and\n-                      all(status \u003d\u003d 404\n-                          for status, _ in failures)):\n+                elif all(status \u003d\u003d 404 for status, _ in failures):\n                     self.logger.warning(\n                         \u0027Update failed because all primaries return 404 \u0027\n```\n\n^ tests pass like this for me.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"777c6f7b2314fa0c560d9d0de0d9501ae094383e","unresolved":false,"context_lines":[{"line_number":911,"context_line":"                if new_successes:"},{"line_number":912,"context_line":"                    update[\u0027successes\u0027] \u003d successes"},{"line_number":913,"context_line":"                    rewrite_pickle \u003d True"},{"line_number":914,"context_line":"                elif (not successes and"},{"line_number":915,"context_line":"                      all(status \u003d\u003d 404"},{"line_number":916,"context_line":"                          for status, _ in failures)):"},{"line_number":917,"context_line":"                    self.logger.warning("}],"source_content_type":"text/x-python","patch_set":3,"id":"4724cb1b_eca605d0","line":914,"range":{"start_line":914,"start_character":22,"end_line":914,"end_character":39},"in_reply_to":"cb33aff3_22f1c17d","updated":"2025-10-25 22:16:10.000000000","message":"Oh, I see now -- you\u0027re right -- I was thinking of `not success` rather than `not successes` (which is a subtle but important distinction!)","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":913,"context_line":"                    rewrite_pickle \u003d True"},{"line_number":914,"context_line":"                elif (not successes and"},{"line_number":915,"context_line":"                      all(status \u003d\u003d 404"},{"line_number":916,"context_line":"                          for status, _ in failures)):"},{"line_number":917,"context_line":"                    self.logger.warning("},{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(path)s %(nodes)s\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"b8fc8a8c_11eeee53","line":916,"updated":"2025-11-05 20:07:32.000000000","message":"ok, so we use `(status, _)` when deciding IF we should log, and `(_, node)` when doing the actual logging.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":918,"context_line":"                        \u0027Update failed because all primaries return 404 \u0027"},{"line_number":919,"context_line":"                        \u0027%(path)s %(nodes)s\u0027,"},{"line_number":920,"context_line":"                        {\u0027path\u0027: path,"},{"line_number":921,"context_line":"                         \u0027nodes\u0027: [node_to_string(node) for _, node in failures]}) # noqa"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"            return rewrite_pickle, redirect"},{"line_number":924,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"b885f58b_0b54cd2d","line":921,"updated":"2025-11-05 20:07:32.000000000","message":"little cheeky noqa for line length - the problem is really that this closure should become a method.\n\nThis node_to_string should use replication\u003dTrue","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":939,"context_line":"        :param op: operation performed (ex: \u0027PUT\u0027 or \u0027DELETE\u0027)"},{"line_number":940,"context_line":"        :param path: /\u003cacct\u003e/\u003ccont\u003e/\u003cobj\u003e path being updated"},{"line_number":941,"context_line":"        :param headers_out: headers to send with the update"},{"line_number":942,"context_line":"        :return: a tuple of (``status``, ``node_id``, ``redirect``)"},{"line_number":943,"context_line":"            where ``status`` is the HTTP status code returned by the container"},{"line_number":944,"context_line":"            server, ``node_id`` is the_id of the node updated and ``redirect``"},{"line_number":945,"context_line":"            is either None or a tuple of (a path, a timestamp string)."}],"source_content_type":"text/x-python","patch_set":3,"id":"797d80d6_058f3944","line":942,"updated":"2025-11-05 20:07:32.000000000","message":"this consistency is a huge win!  KUDOS","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":951,"context_line":"        try:"},{"line_number":952,"context_line":"            with ConnectionTimeout(self.conn_timeout):"},{"line_number":953,"context_line":"                conn \u003d http_connect("},{"line_number":954,"context_line":"                    node[\u0027replication_ip\u0027], node[\u0027replication_port\u0027],"},{"line_number":955,"context_line":"                    node[\u0027device\u0027], part, op, path, headers_out)"},{"line_number":956,"context_line":"            with Timeout(self.node_timeout):"},{"line_number":957,"context_line":"                resp \u003d conn.getresponse()"}],"source_content_type":"text/x-python","patch_set":3,"id":"23cbd641_efb1bc3f","line":954,"updated":"2025-11-05 20:07:32.000000000","message":"since we\u0027re connecting to the node over the replication network that\u0027s the correct way to log it.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":972,"context_line":"                    \u0027Error code %(status)d is returned from remote \u0027"},{"line_number":973,"context_line":"                    \u0027server %(node)s\u0027,"},{"line_number":974,"context_line":"                    {\u0027status\u0027: resp.status,"},{"line_number":975,"context_line":"                     \u0027node\u0027: node_to_string(node, replication\u003dTrue)})"},{"line_number":976,"context_line":"            return status, node[\u0027id\u0027], redirect"},{"line_number":977,"context_line":"        except Exception:"},{"line_number":978,"context_line":"            self.logger.exception(\u0027ERROR with remote server %s\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ed340577_3fc5e89c","line":975,"updated":"2025-11-05 20:07:32.000000000","message":"notice how we use the replication representation of the node for logging here because that\u0027s how we\u0027re connecting to the node.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"}],"test/unit/obj/test_server.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c742470b56ede4196cc0a2ed60a4693c2f592cb9","unresolved":false,"context_lines":[{"line_number":1599,"context_line":"            mock_ring.get_nodes.return_value \u003d (99, [node])"},{"line_number":1600,"context_line":"            object_updater.container_ring \u003d mock_ring"},{"line_number":1601,"context_line":"            mock_update.return_value \u003d ((201, 1, None))"},{"line_number":1602,"context_line":"            object_updater._process_device_in_child(self.sda1, \u0027sda1\u0027)"},{"line_number":1603,"context_line":"        self.assertEqual(1, mock_update.call_count)"},{"line_number":1604,"context_line":"        self.assertEqual((node, 99, \u0027PUT\u0027, \u0027/a/c/o\u0027),"},{"line_number":1605,"context_line":"                         mock_update.call_args_list[0][0][0:4])"}],"source_content_type":"text/x-python","patch_set":2,"id":"46ac9bd1_0e59a2ec","line":1602,"updated":"2025-10-20 04:40:18.000000000","message":"OK, so this isn\u0027t on you, it\u0027s on us: *Why* did [we add object_updater tests to `obj/test_server.py`](https://github.com/openstack/swift/commit/af57922c)??","commit_id":"54d70ea0692630b5347c39298d1a2e24849b735c"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":1587,"context_line":"             \u0027db_state\u0027: \u0027unsharded\u0027})"},{"line_number":1588,"context_line":""},{"line_number":1589,"context_line":"        # verify that only the POST (most recent) async update gets sent by the"},{"line_number":1590,"context_line":"        # object updater, and that both update files are deleted"},{"line_number":1591,"context_line":"        with mock.patch("},{"line_number":1592,"context_line":"            \u0027swift.obj.updater.ObjectUpdater.object_update\u0027) as mock_update, \\"},{"line_number":1593,"context_line":"                mock.patch(\u0027swift.obj.updater.dump_recon_cache\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"31cfca2f_d3c1a9c2","line":1590,"updated":"2025-11-05 20:07:32.000000000","message":"bit of an integration test going here in obj.test_server","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"}],"test/unit/obj/test_updater.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":936,"context_line":"                             normalize_timestamp(0)}},"},{"line_number":937,"context_line":"                        async_pending)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"        bindsock \u003d listen_zero()"},{"line_number":940,"context_line":""},{"line_number":941,"context_line":"        def accepter(sock, return_code):"},{"line_number":942,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"75288ade_020456cc","line":939,"updated":"2025-11-05 20:07:32.000000000","message":"whoa!?\n\nI guess this was actually copied from `test_process_devices_in_child` and AFAIK it hasn\u0027t been causing any problems there.\n\nmaybe better at some level \"just\" to mock `http_connect` from under `object_update` - but this is ... thorough.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":946,"context_line":"                    out.write("},{"line_number":947,"context_line":"                        b\u0027HTTP/1.1 %d Not Found\\r\\nContent-Length: 0\\r\\n\\r\\n\u0027 %"},{"line_number":948,"context_line":"                        return_code)"},{"line_number":949,"context_line":"                    out.flush()"},{"line_number":950,"context_line":"                    self.assertEqual(inc.readline(),"},{"line_number":951,"context_line":"                                     b\u0027PUT /sda1/0/a/c/o HTTP/1.1\\r\\n\u0027)"},{"line_number":952,"context_line":"                    headers \u003d HeaderKeyDict()"}],"source_content_type":"text/x-python","patch_set":3,"id":"77fa82bb_01cf62da","line":949,"updated":"2025-11-05 20:07:32.000000000","message":"... it\u0027s just like... I wouldn\u0027t want to write this code?  I don\u0027t even *really* want to maintain it.  But it\u0027s fine, we can swap it out for an `http_connect` mock if it gets in the way.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"30a500a9140a4d538608ef3302285167d3a46fc7","unresolved":true,"context_lines":[{"line_number":1045,"context_line":"        mock_ring.get_nodes.return_value \u003d ("},{"line_number":1046,"context_line":"            0, [{\u0027id\u0027: 0, \u0027ip\u0027: \u0027127.0.0.2\u0027, \u0027port\u0027: 1, \u0027device\u0027: \u0027sda1\u0027},"},{"line_number":1047,"context_line":"                {\u0027id\u0027: 1, \u0027ip\u0027: \u0027127.0.0.2\u0027, \u0027port\u0027: 1, \u0027device\u0027: \u0027sda1\u0027},"},{"line_number":1048,"context_line":"                {\u0027id\u0027: 2, \u0027ip\u0027: \u0027127.0.0.2\u0027, \u0027port\u0027: 1, \u0027device\u0027: \u0027sda1\u0027}])"},{"line_number":1049,"context_line":"        ou.container_ring \u003d mock_ring"},{"line_number":1050,"context_line":""},{"line_number":1051,"context_line":"        call_count \u003d [0]"}],"source_content_type":"text/x-python","patch_set":3,"id":"41192172_cadaa728","line":1048,"updated":"2025-11-05 20:07:32.000000000","message":"heh, there\u0027s GOT to be some FakeRing pattern in this module we could use to shorten this up some; but I\u0027m all for DAMP, so I\u0027ll allow it 😜\n\n... rolf it turned out the existing RingData that gets saved in setUp is *garbage* and gives all the nodes the same ip 😞 but when I tried to change it a bunch of log line assertions in existing tests on master already assumed we\u0027d get 3 127.0.0.2/sda1 lines so I decided it\u0027s not with it trying to fix just right now.\n\nNew tests could use `ou.container_ring \u003d FakeRing()` if they wanted tho - and IMHO I think that\u0027d be better-ish since it\u0027s a pretty decent/standard way to make sure you get relatively reasonable stubs for the devices in your rings.","commit_id":"16e4c0ac5eb1e4ef7517d4b03a4715ce6c5084b8"}]}