)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"9909c5b346beb678a787795d9197567e3ccc55cb","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Thomas Goirand \u003czigo@debian.org\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-02-10 23:58:15 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"drive-full-checker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The admin documentation provides a documentation on how to \"prevent[ing]"},{"line_number":10,"context_line":"disk full scenarios\" over here:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"d75a48eb_b819c5b5","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":18},"updated":"2024-02-11 16:17:06.000000000","message":"probably something like `swift-drive-space-audit` may be more aligned with the existing tool (swift-drive-audit).","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"990ab8dda6b0f1d6af56bb75b6b696f906366a7b","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Thomas Goirand \u003czigo@debian.org\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-02-10 23:58:15 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"drive-full-checker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The admin documentation provides a documentation on how to \"prevent[ing]"},{"line_number":10,"context_line":"disk full scenarios\" over here:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"f1af0544_a594d079","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":18},"in_reply_to":"d75a48eb_b819c5b5","updated":"2024-02-12 10:25:19.000000000","message":"It\u0027s probably a little late to rename it (a lot of work for not much gain), and I\u0027m not convince drive-space-audit is better than drive-full-checker. If someone else thinks your way, I\u0027ll reconsider. I\u0027ll ask the project team members on IRC what their thoughts are about naming.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"6deb9118372cfcae18899c6ea9b86394aba73275","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Thomas Goirand \u003czigo@debian.org\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-02-10 23:58:15 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"drive-full-checker"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The admin documentation provides a documentation on how to \"prevent[ing]"},{"line_number":10,"context_line":"disk full scenarios\" over here:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"f8c5c4d1_e9dac744","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":18},"in_reply_to":"f1af0544_a594d079","updated":"2024-02-12 13:46:44.000000000","message":"Acknowledged","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":10,"context_line":"disk full scenarios\" over here:"},{"line_number":11,"context_line":"https://docs.openstack.org/swift/latest/admin_guide.html#preventing-disk-full-scenarios"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Even if the doc provides an actual example, this example is written in"},{"line_number":14,"context_line":"Python 2, and its implementation is incomplete."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch intend to fill the gap, and allow administrator to use an"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"5267f5f3_1fc26484","line":13,"range":{"start_line":13,"start_character":8,"end_line":13,"end_character":15},"updated":"2024-03-05 20:20:06.000000000","message":"We should probably update the doc to reference the new tool instead.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":10,"context_line":"disk full scenarios\" over here:"},{"line_number":11,"context_line":"https://docs.openstack.org/swift/latest/admin_guide.html#preventing-disk-full-scenarios"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Even if the doc provides an actual example, this example is written in"},{"line_number":14,"context_line":"Python 2, and its implementation is incomplete."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch intend to fill the gap, and allow administrator to use an"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"bcb09b80_16b95063","line":13,"range":{"start_line":13,"start_character":8,"end_line":13,"end_character":15},"in_reply_to":"5267f5f3_1fc26484","updated":"2024-03-06 08:40:05.000000000","message":"I\u0027d like to write the documentation *AFTER* this patch is merged, in a subsequent patch, if you\u0027re ok with that. That\u0027s because while the patch isn\u0027t merged, the documentation may look different. For example with the {} -\u003e {device} renaming.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a1a16a3a06f13d22116c209922d6bfa8aa5bb097","unresolved":true,"context_lines":[{"line_number":13,"context_line":"Even if the doc provides an actual example, this example is written in"},{"line_number":14,"context_line":"Python 2, and its implementation is incomplete."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch intend to fill the gap, and allow administrator to use an"},{"line_number":17,"context_line":"official implementation of a new \"swift-drive-full-checker\" tool from"},{"line_number":18,"context_line":"/usr/bin directly. Once done, we intend to also patch puppet-swift to"},{"line_number":19,"context_line":"use this new tool."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":34,"id":"40b42e34_0216787e","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":33},"updated":"2024-04-10 14:42:06.000000000","message":"If this is to fill the gap, should be also update the doc somewhat to talk about the presence of this new tool etc? I feel like that might be apart of the scope of adding this?","commit_id":"e51fa1b57e48a1645ac2c213fa71374691d385db"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"9b3b2a981f871a6278284f80688c147d93520072","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"f6d59f64_208bbc1c","updated":"2024-02-10 20:47:24.000000000","message":"recheck","commit_id":"9bc913cf8faf92e20463e64f5c1526155eb298e4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"0237437c_91622b37","updated":"2024-02-12 05:54:52.000000000","message":"Main concerns:\n\n- Lack of conf.d support for rsync configs -- might be able to skip it for now, but I think `\u0026include`s are fairly common. At the very least, we should call it out somewhere; maybe a comment in the sample config around the `rsyncd_conf_path` option?\n- Current behavior when all disks share an rsync module -- I\u0027m pretty sure we want any disk to be able to trigger the shutdown, but as things stand, a single disk (chosen somewhat arbitrarily) decides.\n\nIf needed, I can help with the oslo -\u003e swift.common.utils transition. Let me know if/when I should; I don\u0027t want to step on your toes.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"2d2f8f71a4a11d57d547de190062386310d1cd55","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"bcd37425_1076054f","in_reply_to":"0237437c_91622b37","updated":"2024-02-12 12:30:47.000000000","message":"Hi Tim,\nThe path to rsyncd.conf is configurable, there\u0027s nothing that prevent from setting it to /etc/rcynd.d/something.conf if a user wants to.\n\nThe current behavior is that only the disk that is full will get max connections set to -1, so that\u0027s IMO fine. Of course, that works only with a per-disk rsyncd module type of configuration. I very much recommend anyone to do that anyways.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"4425732fccaa415cecd4b4635f57ce1161ef3f1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"2ddaa49f_c63c005a","updated":"2024-02-21 22:43:33.000000000","message":"recheck","commit_id":"75d1d6d5f017e096558198aaa7aa2c1f3c3ed1e2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"f1155732_a6b49a7c","updated":"2024-03-05 20:20:06.000000000","message":"See what you think of some of https://review.opendev.org/c/openstack/swift/+/911357","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"2299b28b_f6e36466","in_reply_to":"f1155732_a6b49a7c","updated":"2024-03-06 08:40:05.000000000","message":"Thanks a lot for your suggestions. I integrated your changes into this patch, so it can be merged at once.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee81ee0ce0bcd07eaef4c1080f7b56e0c28a9588","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"2c281e9f_0a9f64f7","updated":"2024-03-06 20:16:53.000000000","message":"FWIW, I took a look at how we\u0027re managing this in our clusters. (Sorry, I probably should have checked that a while ago.) It goes something like this:\n\n- All the rsync settings go in `/etc/rsyncd.conf`, with separate modules per-device (named `account_{device}` etc.)\n- At the end of `/etc/rsyncd.conf`, we `\u0026include /etc/rsyncd.d` which can then provide overrides\n- Normally, `/etc/rsyncd.d` is empty. There\u0027s a periodic fullness check that can write (or unlink) `/etc/rsyncd.d/{device}_disabled.conf` that sets `max connections \u003d -1` for a given device\u0027s module\n\nI\u0027d be interested in your thoughts around that approach. I\u0027m *not* saying this needs to look like that, but I see a few benefits:\n\n- `ls /etc/rsyncd.d/` tells you at a glance which devices are currently disabled; throw in a `-l` and you get *when* it was disabled, too\n- There\u0027s only one place on the node telling you what `max connections` *should* be during normal operations -- the checker doesn\u0027t even need to know about it\n- The checker only needs to write during transitions\n- If for some reason you *did* want to expose all disks via a single module, you could do that pretty easily (in fact, we still have some legacy cruft for it that hasn\u0027t been cleaned out) and have the \"any one full disk disables everyone\" semantic I was expecting","commit_id":"5592dad99536408fe2e27133dbfa1aab793bc4f6"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"173b7f88c3309eb0175e2e7219356fd4e6608319","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"e8dd4c77_3f7f0574","in_reply_to":"2c281e9f_0a9f64f7","updated":"2024-03-06 23:02:01.000000000","message":"My colleague Philippe tried this approach, and overriding already installed rsync modules, but unfortunately, writing an rsync module in /etc/rsyncd.d would not override what\u0027s in the main /etc/rsyncd.conf, it simply didn\u0027t work for us. Maybe it\u0027s different in Debian? I\u0027m not sure... but quite sure it wouldn\u0027t work.","commit_id":"5592dad99536408fe2e27133dbfa1aab793bc4f6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"81e4fba5bc108a4e23a80da1af5936094afe1f3a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"0331497e_ac6b1181","in_reply_to":"e8dd4c77_3f7f0574","updated":"2024-03-07 00:41:27.000000000","message":"Interesting... tested on CentOS 7 (rsync 3.1.2) and Ubuntu 22.04 (rsync 3.2.7) and it worked -- though I had to be careful about order. If I put the `\u0026include` *above* the modules, I\u0027d get the behavior you describe, where I couldn\u0027t override:\n```\n$ head -n 100 /etc/rsyncd.conf /etc/rsyncd.d/*\n\u003d\u003d\u003e /etc/rsyncd.conf \u003c\u003d\u003d\nuid\u003dtburke\ngid\u003dtburke\naddress\u003d127.0.0.1\nread only \u003d false\n\n\u0026include /etc/rsyncd.d\n\n[home]\nmax connections \u003d 1\npath \u003d /home/tburke\n\n\u003d\u003d\u003e /etc/rsyncd.d/sdb1_disable.conf \u003c\u003d\u003d\n[home]\nmax connections \u003d -1\n```\nBut by having it *below*, it\u0027d override fine:\n```\n$ head -n 100 /etc/rsyncd.conf /etc/rsyncd.d/*\n\u003d\u003d\u003e /etc/rsyncd.conf \u003c\u003d\u003d\nuid\u003dtburke\ngid\u003dtburke\naddress\u003d127.0.0.1\nread only \u003d false\n\n[home]\nmax connections \u003d 1\npath \u003d /home/tburke\n\n\u0026include /etc/rsyncd.d\n\n\u003d\u003d\u003e /etc/rsyncd.d/sdb1_disable.conf \u003c\u003d\u003d\n[home]\nmax connections \u003d -1\n\n```\nI also tried having the base module defined in another file inside `/etc/rsyncd.d`; then, the sort order of the file names seemed to make the difference.","commit_id":"5592dad99536408fe2e27133dbfa1aab793bc4f6"}],"etc/drive-full-checker.conf-sample":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"39345c6fe489e0c6c574a57225f494782e32dc97","unresolved":true,"context_lines":[{"line_number":6,"context_line":"# rsyncd_conf_path \u003d /etc/rsyncd.conf"},{"line_number":7,"context_line":"#"},{"line_number":8,"context_line":"# You can specify default log routing here if you want:"},{"line_number":9,"context_line":"# log_name \u003d drive-audit"},{"line_number":10,"context_line":"# log_facility \u003d LOG_LOCAL0"},{"line_number":11,"context_line":"# log_level \u003d INFO"},{"line_number":12,"context_line":"# log_address \u003d /dev/log"}],"source_content_type":"application/octet-stream","patch_set":26,"id":"21462a10_82d80d02","line":9,"range":{"start_line":9,"start_character":13,"end_line":9,"end_character":24},"updated":"2024-02-15 12:10:27.000000000","message":"this needs update.","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"7c37a606f3c5953438f711603b32286c1b727fff","unresolved":false,"context_lines":[{"line_number":6,"context_line":"# rsyncd_conf_path \u003d /etc/rsyncd.conf"},{"line_number":7,"context_line":"#"},{"line_number":8,"context_line":"# You can specify default log routing here if you want:"},{"line_number":9,"context_line":"# log_name \u003d drive-audit"},{"line_number":10,"context_line":"# log_facility \u003d LOG_LOCAL0"},{"line_number":11,"context_line":"# log_level \u003d INFO"},{"line_number":12,"context_line":"# log_address \u003d /dev/log"}],"source_content_type":"application/octet-stream","patch_set":26,"id":"1041fe8d_7450b39e","line":9,"range":{"start_line":9,"start_character":13,"end_line":9,"end_character":24},"in_reply_to":"21462a10_82d80d02","updated":"2024-02-15 13:40:27.000000000","message":"Done","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"39345c6fe489e0c6c574a57225f494782e32dc97","unresolved":true,"context_lines":[{"line_number":15,"context_line":"# log_max_line_length \u003d 0"},{"line_number":16,"context_line":"#"},{"line_number":17,"context_line":"# By default, drive-full-checker logs only to syslog. Setting this option True"},{"line_number":18,"context_line":"# makes drive-audit log to console in addition to syslog."},{"line_number":19,"context_line":"# log_to_console \u003d False"},{"line_number":20,"context_line":"#"},{"line_number":21,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":26,"id":"b2efac5a_b000d0d6","line":18,"range":{"start_line":18,"start_character":8,"end_line":18,"end_character":19},"updated":"2024-02-15 12:10:27.000000000","message":"ditto","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"7c37a606f3c5953438f711603b32286c1b727fff","unresolved":false,"context_lines":[{"line_number":15,"context_line":"# log_max_line_length \u003d 0"},{"line_number":16,"context_line":"#"},{"line_number":17,"context_line":"# By default, drive-full-checker logs only to syslog. Setting this option True"},{"line_number":18,"context_line":"# makes drive-audit log to console in addition to syslog."},{"line_number":19,"context_line":"# log_to_console \u003d False"},{"line_number":20,"context_line":"#"},{"line_number":21,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":26,"id":"a6728724_ec3a9e10","line":18,"range":{"start_line":18,"start_character":8,"end_line":18,"end_character":19},"in_reply_to":"b2efac5a_b000d0d6","updated":"2024-02-15 13:40:27.000000000","message":"Done","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":23,"context_line":"#account_max_connections \u003d 8"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# Account server reserved space in GiB. (integer value)"},{"line_number":26,"context_line":"#account_reserved_space \u003d 100"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"# Account section name in the rsyncd.conf file. The \"{}\" sign will be replaced by the drive name. If not using per drive sections, simply"},{"line_number":29,"context_line":"# write \"account\". (string value)"}],"source_content_type":"application/octet-stream","patch_set":31,"id":"06fb4e8f_59250837","line":26,"updated":"2024-03-05 20:20:06.000000000","message":"In your experience, how should this to relate to [`fallocate_reserve`](https://github.com/openstack/swift/blob/master/etc/account-server.conf-sample#L68)? Or would you expect them to be orthogonal?\n\nWould the answer change depending on whether we\u0027re looking at the database layers vs object layer?\n\nWould it be nice to be able to configure this as a percentage, like `fallocate_reserve`?","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":23,"context_line":"#account_max_connections \u003d 8"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# Account server reserved space in GiB. (integer value)"},{"line_number":26,"context_line":"#account_reserved_space \u003d 100"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"# Account section name in the rsyncd.conf file. The \"{}\" sign will be replaced by the drive name. If not using per drive sections, simply"},{"line_number":29,"context_line":"# write \"account\". (string value)"}],"source_content_type":"application/octet-stream","patch_set":31,"id":"8a214a95_8755642e","line":26,"in_reply_to":"06fb4e8f_59250837","updated":"2024-03-06 08:40:05.000000000","message":"I believe both should be in use in production, to avoid for a partition to become full. However, they may have different values. Let me explain.\n\nIn production, the drive-full-checker only runs every N minute. During this interval, an rsync process may start writing to the disk. If between 2 cron job runs, some rsync fill-up the disk, then it\u0027s game over. So my suggestion for production, is to this the reserved_space in disk-full-checker to this formula:\n\namount-of-data-the-disk-can-write-per-minute\nx\nnumber-of-minutes-between-drive-full-checker-cron-job\nx\nproduction-value-for-max-connections-in-rsyncd.conf\n+\na margin value (maybe the above x2 ? that\u0027s what we did...)\n\nTherefore, typically, the value of {a,c,o}__reserved_space must be set to a higher value than the fallocate_reserve, and contrary to fallocate_reserve, it depends on how fast the drives are writing data.\n\nBecause of the above, I don\u0027t really see the point of having a percent. A percent value doesn\u0027t include any kind of smart calculation of how fast the drives are filling-up, which is hopefully what a smart admin will calculate prior using drive-full-checker.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":25,"context_line":"# Account server reserved space in GiB. (integer value)"},{"line_number":26,"context_line":"#account_reserved_space \u003d 100"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"# Account section name in the rsyncd.conf file. The \"{}\" sign will be replaced by the drive name. If not using per drive sections, simply"},{"line_number":29,"context_line":"# write \"account\". (string value)"},{"line_number":30,"context_line":"#account_rsyncd_section_name \u003d \" account_{} \""},{"line_number":31,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":31,"id":"da2cbcad_e01ab5e0","line":28,"range":{"start_line":28,"start_character":53,"end_line":28,"end_character":55},"updated":"2024-03-05 20:20:06.000000000","message":"We might want to use `{device}`, similar to what we do with the rsync modules in [rsyncd.conf-sample](https://github.com/openstack/swift/blob/master/etc/rsyncd.conf-sample) and [object-server.conf-sample](https://github.com/openstack/swift/blob/master/etc/object-server.conf-sample)","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":25,"context_line":"# Account server reserved space in GiB. (integer value)"},{"line_number":26,"context_line":"#account_reserved_space \u003d 100"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"# Account section name in the rsyncd.conf file. The \"{}\" sign will be replaced by the drive name. If not using per drive sections, simply"},{"line_number":29,"context_line":"# write \"account\". (string value)"},{"line_number":30,"context_line":"#account_rsyncd_section_name \u003d \" account_{} \""},{"line_number":31,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":31,"id":"7a7d6baf_5b5239e1","line":28,"range":{"start_line":28,"start_character":53,"end_line":28,"end_character":55},"in_reply_to":"da2cbcad_e01ab5e0","updated":"2024-03-06 08:40:05.000000000","message":"Done as I incorporated your changes from #911357.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"}],"swift/cli/drive_full_checker.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"ef2521bf40d41710280474c0fa8356fe71506fd4","unresolved":true,"context_lines":[{"line_number":27,"context_line":"from oslo_config import cfg"},{"line_number":28,"context_line":"from oslo_log import log as logging"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__, project\u003d\u0027swift-drive-full-checker\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"GiB \u003d 1024 * 1024 * 1024"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"4a7c8026_60791dc6","line":30,"updated":"2024-02-11 16:16:13.000000000","message":"afaik swift intentionally does not use oslo libraries. So using oslo.config or oslo.log may be unnaaceptable.\n\nPlease check how bin/swift-drive-audit is implemented and follow that pattern.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":27,"context_line":"from oslo_config import cfg"},{"line_number":28,"context_line":"from oslo_log import log as logging"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__, project\u003d\u0027swift-drive-full-checker\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"GiB \u003d 1024 * 1024 * 1024"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"f90d0650_9c2731f0","line":30,"in_reply_to":"4a7c8026_60791dc6","updated":"2024-02-12 05:54:52.000000000","message":"FWIW, there\u0027s a `swift.common.utils.get_logger` that would likely be helpful here in particular.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"990ab8dda6b0f1d6af56bb75b6b696f906366a7b","unresolved":false,"context_lines":[{"line_number":27,"context_line":"from oslo_config import cfg"},{"line_number":28,"context_line":"from oslo_log import log as logging"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d logging.getLogger(__name__, project\u003d\u0027swift-drive-full-checker\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"GiB \u003d 1024 * 1024 * 1024"},{"line_number":33,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"0c85741b_929107f7","line":30,"in_reply_to":"f90d0650_9c2731f0","updated":"2024-02-12 10:25:19.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"ef2521bf40d41710280474c0fa8356fe71506fd4","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Initialize script common oslo_config"},{"line_number":35,"context_line":"common_opts \u003d ["},{"line_number":36,"context_line":"    cfg.IntOpt(\u0027account_max_connections\u0027, short\u003d\u0027b\u0027, default\u003d8,"},{"line_number":37,"context_line":"               help\u003d\u0027Max connections to the Account rsync backend.\u0027),"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    cfg.IntOpt(\u0027account_reserved_space\u0027, short\u003d\u0027a\u0027, default\u003d100,"}],"source_content_type":"text/x-python","patch_set":19,"id":"c86c5c7a_234fed95","line":36,"range":{"start_line":36,"start_character":42,"end_line":36,"end_character":52},"updated":"2024-02-11 16:16:13.000000000","message":"What\u0027s the point of allowing configuration via config file as well as argument ? I don\u0027t think there may be valid use case to use args (or mixing both args and options).\n\nI\u0027d suggest just relying on config options instead.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"990ab8dda6b0f1d6af56bb75b6b696f906366a7b","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Initialize script common oslo_config"},{"line_number":35,"context_line":"common_opts \u003d ["},{"line_number":36,"context_line":"    cfg.IntOpt(\u0027account_max_connections\u0027, short\u003d\u0027b\u0027, default\u003d8,"},{"line_number":37,"context_line":"               help\u003d\u0027Max connections to the Account rsync backend.\u0027),"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    cfg.IntOpt(\u0027account_reserved_space\u0027, short\u003d\u0027a\u0027, default\u003d100,"}],"source_content_type":"text/x-python","patch_set":19,"id":"849426e1_0d41204a","line":36,"range":{"start_line":36,"start_character":42,"end_line":36,"end_character":52},"in_reply_to":"4b90097b_2befa75e","updated":"2024-02-12 10:25:19.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"# Initialize script common oslo_config"},{"line_number":35,"context_line":"common_opts \u003d ["},{"line_number":36,"context_line":"    cfg.IntOpt(\u0027account_max_connections\u0027, short\u003d\u0027b\u0027, default\u003d8,"},{"line_number":37,"context_line":"               help\u003d\u0027Max connections to the Account rsync backend.\u0027),"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    cfg.IntOpt(\u0027account_reserved_space\u0027, short\u003d\u0027a\u0027, default\u003d100,"}],"source_content_type":"text/x-python","patch_set":19,"id":"4b90097b_2befa75e","line":36,"range":{"start_line":36,"start_character":42,"end_line":36,"end_character":52},"in_reply_to":"c86c5c7a_234fed95","updated":"2024-02-12 05:54:52.000000000","message":"Often we (swift operators) find some value in cli args that override configs -- it enables things like one-off replicator runs to move a single partition, for example. But I think I agree that this particular tool may not benefit from it much -- or, if operators *do* run it one-off with overridden configs, they may be surprised to have the results reversed the next time it runs from cron.\n\nAssuming we go down the `swift.common.utils.readconf` path (which doesn\u0027t come with automatic support for such overrides), I\u0027d say leave them out -- YAGNI. If someone comes up with a sane use for overrides, they can propose it separately after the new tool has landed.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":74,"context_line":"               help\u003d\u0027Mount point of your storage.\u0027),"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    cfg.StrOpt(\u0027rsyncd_conf_path\u0027, short\u003d\u0027s\u0027, default\u003d\u0027/etc/rsyncd.conf\u0027,"},{"line_number":77,"context_line":"               help\u003d\u0027Mount point of your storage.\u0027),"},{"line_number":78,"context_line":"]"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"94485865_5b0b8bf8","line":77,"range":{"start_line":77,"start_character":21,"end_line":77,"end_character":48},"updated":"2024-02-12 05:54:52.000000000","message":"\"Rsync configuration file\" surely ;-)","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"2d2f8f71a4a11d57d547de190062386310d1cd55","unresolved":false,"context_lines":[{"line_number":74,"context_line":"               help\u003d\u0027Mount point of your storage.\u0027),"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    cfg.StrOpt(\u0027rsyncd_conf_path\u0027, short\u003d\u0027s\u0027, default\u003d\u0027/etc/rsyncd.conf\u0027,"},{"line_number":77,"context_line":"               help\u003d\u0027Mount point of your storage.\u0027),"},{"line_number":78,"context_line":"]"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"2c956ba2_a41f17c3","line":77,"range":{"start_line":77,"start_character":21,"end_line":77,"end_character":48},"in_reply_to":"94485865_5b0b8bf8","updated":"2024-02-12 12:30:47.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        with open(rsyncd_p, \u0027r\u0027) as f:"},{"line_number":96,"context_line":"            file_content \u003d fake_section + f.read()"},{"line_number":97,"context_line":"    except Exception as err:"},{"line_number":98,"context_line":"        print(\"Unexpected error reading {}: {}\".format(rsyncd_p, err))"},{"line_number":99,"context_line":"        return 1"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    cp \u003d configparser.RawConfigParser()"}],"source_content_type":"text/x-python","patch_set":19,"id":"b6002c81_725ffacb","line":98,"updated":"2024-02-12 05:54:52.000000000","message":"I trip this in my dev env, on a line like\n```\n\u0026include /etc/rsyncd.d\n```\nUnfortunately, I think this is a fairly common way of deploying rsync for swift -- have a central `/etc/rsyncd.conf` with some global options (`uid`, `gid`, `address`, `log file`, etc.) then a config dir full of per-service, per-device files with one section each defining things like `path`, `lock file`, and `max connections`.\n\nI guess maybe we just need to make sure we\u0027ve set expectations appropriately with a note about how you (currently) can only use it if you\u0027ve got a single rsync file?","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"2d2f8f71a4a11d57d547de190062386310d1cd55","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        with open(rsyncd_p, \u0027r\u0027) as f:"},{"line_number":96,"context_line":"            file_content \u003d fake_section + f.read()"},{"line_number":97,"context_line":"    except Exception as err:"},{"line_number":98,"context_line":"        print(\"Unexpected error reading {}: {}\".format(rsyncd_p, err))"},{"line_number":99,"context_line":"        return 1"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    cp \u003d configparser.RawConfigParser()"}],"source_content_type":"text/x-python","patch_set":19,"id":"e8f364db_1edcc841","line":98,"in_reply_to":"3dc523c4_b7d4fdb5","updated":"2024-02-12 12:30:47.000000000","message":"It\u0027d probably be nice to be able to use multiple files like Tim wrote. Maybe we can add the feature once the first version of the tool is merged. However, at present, we don\u0027t need this here at $work, so maybe I\u0027ll let someone else contribute it?","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":36116,"name":"Philippe SÉRAPHIN","email":"philippe.seraphin@infomaniak.com","username":"pseraf"},"change_message_id":"7bbe218ec0ece01621efc3f78439501ad61319a7","unresolved":true,"context_lines":[{"line_number":95,"context_line":"        with open(rsyncd_p, \u0027r\u0027) as f:"},{"line_number":96,"context_line":"            file_content \u003d fake_section + f.read()"},{"line_number":97,"context_line":"    except Exception as err:"},{"line_number":98,"context_line":"        print(\"Unexpected error reading {}: {}\".format(rsyncd_p, err))"},{"line_number":99,"context_line":"        return 1"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    cp \u003d configparser.RawConfigParser()"}],"source_content_type":"text/x-python","patch_set":19,"id":"3dc523c4_b7d4fdb5","line":98,"in_reply_to":"b6002c81_725ffacb","updated":"2024-02-12 06:43:24.000000000","message":"I worked with Thomasand i tested this solution.\nBut, if max_connections is primary configured for a module in /etc/rsyncd.conf, it cannot be override by /etc/rsync.d/***.conf","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"fc2668c3cc0c6f646dd8bdd8d0dd8b643371226d","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        with open(rsyncd_p, \u0027r\u0027) as f:"},{"line_number":96,"context_line":"            file_content \u003d fake_section + f.read()"},{"line_number":97,"context_line":"    except Exception as err:"},{"line_number":98,"context_line":"        print(\"Unexpected error reading {}: {}\".format(rsyncd_p, err))"},{"line_number":99,"context_line":"        return 1"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    cp \u003d configparser.RawConfigParser()"}],"source_content_type":"text/x-python","patch_set":19,"id":"180143ed_ecf1a494","line":98,"in_reply_to":"e8f364db_1edcc841","updated":"2024-02-12 13:46:06.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":105,"context_line":"        cp.read_string(file_content)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    # For all dirs in /srv/node"},{"line_number":108,"context_line":"    for dir in os.listdir(storage_p):"},{"line_number":109,"context_line":"        dirpath \u003d os.path.join(storage_p, dir)"},{"line_number":110,"context_line":"        # If the dir is mounted"},{"line_number":111,"context_line":"        if os.path.ismount(dirpath):"}],"source_content_type":"text/x-python","patch_set":19,"id":"bf8241c6_036b37a2","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":11},"updated":"2024-02-12 05:54:52.000000000","message":"Better as `dev`, to avoid shadowing the built-in `dir`","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"2d2f8f71a4a11d57d547de190062386310d1cd55","unresolved":true,"context_lines":[{"line_number":105,"context_line":"        cp.read_string(file_content)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    # For all dirs in /srv/node"},{"line_number":108,"context_line":"    for dir in os.listdir(storage_p):"},{"line_number":109,"context_line":"        dirpath \u003d os.path.join(storage_p, dir)"},{"line_number":110,"context_line":"        # If the dir is mounted"},{"line_number":111,"context_line":"        if os.path.ismount(dirpath):"}],"source_content_type":"text/x-python","patch_set":19,"id":"e7daa1e6_214d7342","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":11},"in_reply_to":"bf8241c6_036b37a2","updated":"2024-02-12 12:30:47.000000000","message":"I ended-up using \"srvnode_dir\" instead.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"fc2668c3cc0c6f646dd8bdd8d0dd8b643371226d","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        cp.read_string(file_content)"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    # For all dirs in /srv/node"},{"line_number":108,"context_line":"    for dir in os.listdir(storage_p):"},{"line_number":109,"context_line":"        dirpath \u003d os.path.join(storage_p, dir)"},{"line_number":110,"context_line":"        # If the dir is mounted"},{"line_number":111,"context_line":"        if os.path.ismount(dirpath):"}],"source_content_type":"text/x-python","patch_set":19,"id":"75f99bcc_55c73385","line":108,"range":{"start_line":108,"start_character":8,"end_line":108,"end_character":11},"in_reply_to":"e7daa1e6_214d7342","updated":"2024-02-12 13:46:06.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":108,"context_line":"    for dir in os.listdir(storage_p):"},{"line_number":109,"context_line":"        dirpath \u003d os.path.join(storage_p, dir)"},{"line_number":110,"context_line":"        # If the dir is mounted"},{"line_number":111,"context_line":"        if os.path.ismount(dirpath):"},{"line_number":112,"context_line":"            # shutil.disk_usage can be mocked in tests."},{"line_number":113,"context_line":"            if sys.version_info[0] \u003d\u003d 2:"},{"line_number":114,"context_line":"                space \u003d os.statvfs(dirpath)"}],"source_content_type":"text/x-python","patch_set":19,"id":"b7125bc5_2a194fba","line":111,"updated":"2024-02-12 05:54:52.000000000","message":"Maybe use `swift.common.utils.ismount` -- there are some weird special cases where we might want to treat it as a mountpoint even if it\u0027s not really one; see https://github.com/openstack/swift/commit/5eeaa95\n\nMight also want to change it to\n```\nif not ismount(dirpath):\n    continue\n```\nand save some indent, but that\u0027s more of a nit -- definitely not a blocker.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"fc2668c3cc0c6f646dd8bdd8d0dd8b643371226d","unresolved":false,"context_lines":[{"line_number":108,"context_line":"    for dir in os.listdir(storage_p):"},{"line_number":109,"context_line":"        dirpath \u003d os.path.join(storage_p, dir)"},{"line_number":110,"context_line":"        # If the dir is mounted"},{"line_number":111,"context_line":"        if os.path.ismount(dirpath):"},{"line_number":112,"context_line":"            # shutil.disk_usage can be mocked in tests."},{"line_number":113,"context_line":"            if sys.version_info[0] \u003d\u003d 2:"},{"line_number":114,"context_line":"                space \u003d os.statvfs(dirpath)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5ac2963e_eb74fcc6","line":111,"in_reply_to":"b7125bc5_2a194fba","updated":"2024-02-12 13:46:06.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":166,"context_line":"                            LOG.warning(\u0027Enabling \u0027 + search_str)"},{"line_number":167,"context_line":"                        cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":168,"context_line":"            except KeyError:"},{"line_number":169,"context_line":"                pass"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            # Same for object"},{"line_number":172,"context_line":"            if \u0027{}\u0027 in object_secname:"}],"source_content_type":"text/x-python","patch_set":19,"id":"5e5d0616_7576a5ba","line":169,"updated":"2024-02-12 05:54:52.000000000","message":"The repetition seems to beg for a helper function.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"fc2668c3cc0c6f646dd8bdd8d0dd8b643371226d","unresolved":false,"context_lines":[{"line_number":166,"context_line":"                            LOG.warning(\u0027Enabling \u0027 + search_str)"},{"line_number":167,"context_line":"                        cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":168,"context_line":"            except KeyError:"},{"line_number":169,"context_line":"                pass"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"            # Same for object"},{"line_number":172,"context_line":"            if \u0027{}\u0027 in object_secname:"}],"source_content_type":"text/x-python","patch_set":19,"id":"7e8ca20b_67b2be9f","line":169,"in_reply_to":"5e5d0616_7576a5ba","updated":"2024-02-12 13:46:06.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"47b295cbcb27c52c08bc80a93559802acb6bee8b","unresolved":true,"context_lines":[{"line_number":172,"context_line":"            if \u0027{}\u0027 in object_secname:"},{"line_number":173,"context_line":"                search_str \u003d object_secname.format(dir)"},{"line_number":174,"context_line":"            else:"},{"line_number":175,"context_line":"                search_str \u003d object_secname"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"            try:"},{"line_number":178,"context_line":"                if cp[search_str]:"}],"source_content_type":"text/x-python","patch_set":19,"id":"8b1a3532_3b885521","line":175,"updated":"2024-02-12 05:54:52.000000000","message":"So if all drives in a box use the same rsync module, and one drive exceeds the threshold -- we want to disable it, right? But as it is, won\u0027t it depend on the order of the `listdir`, and we\u0027ll just base our decision off the the last drive?","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"2d2f8f71a4a11d57d547de190062386310d1cd55","unresolved":true,"context_lines":[{"line_number":172,"context_line":"            if \u0027{}\u0027 in object_secname:"},{"line_number":173,"context_line":"                search_str \u003d object_secname.format(dir)"},{"line_number":174,"context_line":"            else:"},{"line_number":175,"context_line":"                search_str \u003d object_secname"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"            try:"},{"line_number":178,"context_line":"                if cp[search_str]:"}],"source_content_type":"text/x-python","patch_set":19,"id":"ebda86e9_087b58e8","line":175,"in_reply_to":"8b1a3532_3b885521","updated":"2024-02-12 12:30:47.000000000","message":"Id\u0027d say to this that we should recommend using one rsync module per drive if using this tool.","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"fc2668c3cc0c6f646dd8bdd8d0dd8b643371226d","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            if \u0027{}\u0027 in object_secname:"},{"line_number":173,"context_line":"                search_str \u003d object_secname.format(dir)"},{"line_number":174,"context_line":"            else:"},{"line_number":175,"context_line":"                search_str \u003d object_secname"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"            try:"},{"line_number":178,"context_line":"                if cp[search_str]:"}],"source_content_type":"text/x-python","patch_set":19,"id":"aed883a2_9ba1474b","line":175,"in_reply_to":"ebda86e9_087b58e8","updated":"2024-02-12 13:46:06.000000000","message":"Done","commit_id":"57624ad62f1529135880bf774de85a47960a9620"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a6f3f266241d8bad887ff76190a3049aa18087f1","unresolved":true,"context_lines":[{"line_number":24,"context_line":"import shutil"},{"line_number":25,"context_line":"import sys"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"from six.moves.configparser import ConfigParser"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from swift.common.utils import config_true_value, ismount, get_logger"}],"source_content_type":"text/x-python","patch_set":26,"id":"bde8e0d6_ca93b4eb","line":27,"updated":"2024-02-15 12:09:50.000000000","message":"one line can be removed.","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"4ef923794beb93f320d417b88b7277f4d82f9a34","unresolved":false,"context_lines":[{"line_number":24,"context_line":"import shutil"},{"line_number":25,"context_line":"import sys"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"from six.moves.configparser import ConfigParser"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from swift.common.utils import config_true_value, ismount, get_logger"}],"source_content_type":"text/x-python","patch_set":26,"id":"cdb59160_c1364eca","line":27,"in_reply_to":"bde8e0d6_ca93b4eb","updated":"2024-02-15 14:27:35.000000000","message":"Done","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a6f3f266241d8bad887ff76190a3049aa18087f1","unresolved":true,"context_lines":[{"line_number":63,"context_line":"            else:"},{"line_number":64,"context_line":"                cm \u003d mc"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"            if int(cp[search_str][\u0027max connections\u0027]) !\u003d cm:"},{"line_number":67,"context_line":"                if cm \u003d\u003d -1:"},{"line_number":68,"context_line":"                    logger.info(\u0027Disabling \u0027 + search_str)"},{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"f29a5bc8_74be67aa","line":71,"range":{"start_line":66,"start_character":12,"end_line":71,"end_character":59},"updated":"2024-02-15 12:09:50.000000000","message":"are we sure this works correctly in case \n\n - rsync is not per device\n - sdb is full but sdc has enough space","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"4ef923794beb93f320d417b88b7277f4d82f9a34","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            else:"},{"line_number":64,"context_line":"                cm \u003d mc"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"            if int(cp[search_str][\u0027max connections\u0027]) !\u003d cm:"},{"line_number":67,"context_line":"                if cm \u003d\u003d -1:"},{"line_number":68,"context_line":"                    logger.info(\u0027Disabling \u0027 + search_str)"},{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"},{"line_number":74,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"68c16b10_03b5487c","line":71,"range":{"start_line":66,"start_character":12,"end_line":71,"end_character":59},"in_reply_to":"f29a5bc8_74be67aa","updated":"2024-02-15 14:27:35.000000000","message":"I\u0027m quite sure this scenario (ie: a single rsync module per service) is untested, and what I\u0027m guessing thinking about how I wrote it, that it\u0027s going to be a kind of random result, where the last drive in the for srvnode_dir in os.listdir that \"wins\".\n\nSo I\u0027m planning to explain this in the doc, and strongly recommend against using a single rsync module per service. If you have suggestions for improvement on how to do this so it works on a single rsync module, let me know, but IMO, it doesn\u0027t make sense to prevent writing on a node only because a single drive is full anyways.\n\nBTW, I\u0027m planning to write the doc after this patch is merged, in a separate commit.","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a6f3f266241d8bad887ff76190a3049aa18087f1","unresolved":true,"context_lines":[{"line_number":139,"context_line":"    # disk-full-checker config file parsing"},{"line_number":140,"context_line":"    c \u003d ConfigParser()"},{"line_number":141,"context_line":"    source_file \u003d None"},{"line_number":142,"context_line":"    try:"},{"line_number":143,"context_line":"        if len(sys.argv) \u003e 2 and sys.argv[2] \u003d\u003d \u0027--source-file\u0027:"},{"line_number":144,"context_line":"            source_file \u003d sys.argv[3]"},{"line_number":145,"context_line":"        conf_path \u003d sys.argv[1]"},{"line_number":146,"context_line":"    except Exception:"},{"line_number":147,"context_line":"        print(\"Usage: %s CONF_FILE [--source-file \u003csrc-file\u003e]\""},{"line_number":148,"context_line":"              % sys.argv[0].split(\u0027/\u0027)[-1])"},{"line_number":149,"context_line":"        sys.exit(1)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    if not c.read(conf_path):"},{"line_number":152,"context_line":"        print(\"Unable to read config file %s\" % conf_path)"}],"source_content_type":"text/x-python","patch_set":26,"id":"2d26ca39_1c1a97a8","line":149,"range":{"start_line":142,"start_character":4,"end_line":149,"end_character":19},"updated":"2024-02-15 12:09:50.000000000","message":"You can use argparse.ArgumentParser","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"4ef923794beb93f320d417b88b7277f4d82f9a34","unresolved":false,"context_lines":[{"line_number":139,"context_line":"    # disk-full-checker config file parsing"},{"line_number":140,"context_line":"    c \u003d ConfigParser()"},{"line_number":141,"context_line":"    source_file \u003d None"},{"line_number":142,"context_line":"    try:"},{"line_number":143,"context_line":"        if len(sys.argv) \u003e 2 and sys.argv[2] \u003d\u003d \u0027--source-file\u0027:"},{"line_number":144,"context_line":"            source_file \u003d sys.argv[3]"},{"line_number":145,"context_line":"        conf_path \u003d sys.argv[1]"},{"line_number":146,"context_line":"    except Exception:"},{"line_number":147,"context_line":"        print(\"Usage: %s CONF_FILE [--source-file \u003csrc-file\u003e]\""},{"line_number":148,"context_line":"              % sys.argv[0].split(\u0027/\u0027)[-1])"},{"line_number":149,"context_line":"        sys.exit(1)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    if not c.read(conf_path):"},{"line_number":152,"context_line":"        print(\"Unable to read config file %s\" % conf_path)"}],"source_content_type":"text/x-python","patch_set":26,"id":"f79cd5ae_51cffc62","line":149,"range":{"start_line":142,"start_character":4,"end_line":149,"end_character":19},"in_reply_to":"2d26ca39_1c1a97a8","updated":"2024-02-15 14:27:35.000000000","message":"Done","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a6f3f266241d8bad887ff76190a3049aa18087f1","unresolved":true,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    # logging facility setup"},{"line_number":176,"context_line":"    log_to_console \u003d config_true_value(CONF.get(\u0027log_to_console\u0027, False))"},{"line_number":177,"context_line":"    CONF[\u0027log_name\u0027] \u003d CONF.get(\u0027log_name\u0027, \u0027drive-full-checker\u0027)"},{"line_number":178,"context_line":"    logger \u003d get_logger(CONF, log_to_console\u003dlog_to_console,"},{"line_number":179,"context_line":"                        log_route\u003d\u0027drive-full-checker\u0027)"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"22ee5cf1_5dc9267e","line":177,"range":{"start_line":177,"start_character":45,"end_line":177,"end_character":63},"updated":"2024-02-15 12:09:50.000000000","message":"(nit) I wonder if we want to make the name of this tool more consistent with drive-audit, like, `drive-space-audit`","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"4ef923794beb93f320d417b88b7277f4d82f9a34","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    # logging facility setup"},{"line_number":176,"context_line":"    log_to_console \u003d config_true_value(CONF.get(\u0027log_to_console\u0027, False))"},{"line_number":177,"context_line":"    CONF[\u0027log_name\u0027] \u003d CONF.get(\u0027log_name\u0027, \u0027drive-full-checker\u0027)"},{"line_number":178,"context_line":"    logger \u003d get_logger(CONF, log_to_console\u003dlog_to_console,"},{"line_number":179,"context_line":"                        log_route\u003d\u0027drive-full-checker\u0027)"},{"line_number":180,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"edfc4194_589787a2","line":177,"range":{"start_line":177,"start_character":45,"end_line":177,"end_character":63},"in_reply_to":"22ee5cf1_5dc9267e","updated":"2024-02-15 14:27:35.000000000","message":"I like \"drive-full-checker\" better, even if it isn\u0027t consistent with \"drive-audit\".","commit_id":"b9461e9e02ab62bc2fff8e293f2420b220cd13ad"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":32,"context_line":"GiB \u003d 1024 * 1024 * 1024"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"# Params for this function:"},{"line_number":36,"context_line":"# logger: ref to the logger"},{"line_number":37,"context_line":"# cp: config parser object containing the rsyncd.conf representation"},{"line_number":38,"context_line":"# srvnode_dir: name of the drive we\u0027re inspecting (for example: sdb)"}],"source_content_type":"text/x-python","patch_set":31,"id":"8289bc12_a6dd98f3","line":35,"updated":"2024-03-05 20:20:06.000000000","message":"We might want to move all this into a proper docstring.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":32,"context_line":"GiB \u003d 1024 * 1024 * 1024"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"# Params for this function:"},{"line_number":36,"context_line":"# logger: ref to the logger"},{"line_number":37,"context_line":"# cp: config parser object containing the rsyncd.conf representation"},{"line_number":38,"context_line":"# srvnode_dir: name of the drive we\u0027re inspecting (for example: sdb)"}],"source_content_type":"text/x-python","patch_set":31,"id":"ff59295b_30261c86","line":35,"in_reply_to":"8289bc12_a6dd98f3","updated":"2024-03-06 08:40:05.000000000","message":"Done as I incorporated your changes from #911357.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":41,"context_line":"# rs: bytes reserved space in srvnode_dir"},{"line_number":42,"context_line":"# mc: \"normal\" max connections (ie: when partition isn\u0027t full)"},{"line_number":43,"context_line":"#     for the given dir entry"},{"line_number":44,"context_line":"def _patch_rsyncdconf_entry(logger, cp, srvnode_dir, free, sec_name, rs, mc):"},{"line_number":45,"context_line":"    # Calculate section name (ie: replace \u0027{}\u0027 by drive name if present)"},{"line_number":46,"context_line":"    if \u0027{}\u0027 in sec_name:"},{"line_number":47,"context_line":"        search_str \u003d sec_name.format(srvnode_dir).strip(\u0027\"\u0027)"}],"source_content_type":"text/x-python","patch_set":31,"id":"250117ce_a9f8655e","line":44,"range":{"start_line":44,"start_character":69,"end_line":44,"end_character":75},"updated":"2024-03-05 20:20:06.000000000","message":"I might make these a little more descriptive ;-)","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":41,"context_line":"# rs: bytes reserved space in srvnode_dir"},{"line_number":42,"context_line":"# mc: \"normal\" max connections (ie: when partition isn\u0027t full)"},{"line_number":43,"context_line":"#     for the given dir entry"},{"line_number":44,"context_line":"def _patch_rsyncdconf_entry(logger, cp, srvnode_dir, free, sec_name, rs, mc):"},{"line_number":45,"context_line":"    # Calculate section name (ie: replace \u0027{}\u0027 by drive name if present)"},{"line_number":46,"context_line":"    if \u0027{}\u0027 in sec_name:"},{"line_number":47,"context_line":"        search_str \u003d sec_name.format(srvnode_dir).strip(\u0027\"\u0027)"}],"source_content_type":"text/x-python","patch_set":31,"id":"9eb70709_ed3c7922","line":44,"range":{"start_line":44,"start_character":69,"end_line":44,"end_character":75},"in_reply_to":"250117ce_a9f8655e","updated":"2024-03-06 08:40:05.000000000","message":"Done as I incorporated your changes from #911357.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    # If referenced in the rsyncd.conf"},{"line_number":52,"context_line":"    # In old setup (Python 2), calling config_parser[\u0027something\u0027]"},{"line_number":53,"context_line":"    # raises an exception if the something section is not present"},{"line_number":54,"context_line":"    # in the config file. Which is why we must do try/except."},{"line_number":55,"context_line":"    # I believe this try/except can be removed on more recent"},{"line_number":56,"context_line":"    # Python3 based setups."}],"source_content_type":"text/x-python","patch_set":31,"id":"0e95b76c_9fb6d975","line":53,"updated":"2024-03-05 20:20:06.000000000","message":"Maybe we could use `has_section` instead?","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    # If referenced in the rsyncd.conf"},{"line_number":52,"context_line":"    # In old setup (Python 2), calling config_parser[\u0027something\u0027]"},{"line_number":53,"context_line":"    # raises an exception if the something section is not present"},{"line_number":54,"context_line":"    # in the config file. Which is why we must do try/except."},{"line_number":55,"context_line":"    # I believe this try/except can be removed on more recent"},{"line_number":56,"context_line":"    # Python3 based setups."}],"source_content_type":"text/x-python","patch_set":31,"id":"fb70a1ec_f1f5d58e","line":53,"in_reply_to":"0e95b76c_9fb6d975","updated":"2024-03-06 08:40:05.000000000","message":"Done as I incorporated your changes from #911357.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                if cm \u003d\u003d -1:"},{"line_number":68,"context_line":"                    logger.info(\u0027Disabling \u0027 + search_str)"},{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":31,"id":"65b4a59e_9504675b","line":70,"updated":"2024-03-05 20:20:06.000000000","message":"We\u0027ll also trip this if someone one-off tweaks it to be something other than the `max_connections` from our new config.\n\nNot opposed; just mentioning.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    return cp"}],"source_content_type":"text/x-python","patch_set":31,"id":"e73a2a09_bbb25991","line":72,"updated":"2024-03-05 20:20:06.000000000","message":"OK, and this covers both missing section *and* missing option, yeah?\n\nShould we maybe do something like `cp[search_str].get(\u0027max connections\u0027, 0)` to match [rsync\u0027s default](https://www.man7.org/linux/man-pages/man5/rsyncd.conf.5.html)?\n\n\u003e This parameter allows you to specify the maximum number of simultaneous connections you will allow.  Any clients connecting when the maximum has been reached will receive a message telling them to try later.  The default is 0, which means no limit.  A negative value disables the module.  See also the \"lock file\" parameter.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"173b7f88c3309eb0175e2e7219356fd4e6608319","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    return cp"}],"source_content_type":"text/x-python","patch_set":31,"id":"499d9ae6_64657885","line":72,"in_reply_to":"757c16ea_b5df04c4","updated":"2024-03-06 23:02:01.000000000","message":"If there is a missing option in drive-full-checker.conf, it\u0027s going to be ok because of defaults like in:\n\naccount_max_connections \u003d int(conf.get(\u0027account_max_connections\u0027, 8))\n\nWe could require a positive int indeed, but what if someone wants to, let\u0027s say with puppet, cluster wide, write -1 everywhere, to disable rsync? Imposing a strictly positive int would make this not possible. I think it\u0027s nicer to just let our users do what they want.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee81ee0ce0bcd07eaef4c1080f7b56e0c28a9588","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    return cp"}],"source_content_type":"text/x-python","patch_set":31,"id":"757c16ea_b5df04c4","line":72,"in_reply_to":"8e6c6dcf_87b7873a","updated":"2024-03-06 20:16:53.000000000","message":"\u003e In production, I would strongly recommend against using 0 in max connections. This would dramatically kill any kind of operation on the drive because of too many rsync processes.\n\nAbsolutely, makes perfect sense! I was mainly thinking about how to handle a missing option when reading the config. Maybe we should require that `*_rsync_max_connections` be a *positive* int?","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                else:"},{"line_number":70,"context_line":"                    logger.info(\u0027Enabling \u0027 + search_str)"},{"line_number":71,"context_line":"                cp[search_str][\u0027max connections\u0027] \u003d str(cm)"},{"line_number":72,"context_line":"    except KeyError:"},{"line_number":73,"context_line":"        pass"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    return cp"}],"source_content_type":"text/x-python","patch_set":31,"id":"8e6c6dcf_87b7873a","line":72,"in_reply_to":"e73a2a09_bbb25991","updated":"2024-03-06 08:40:05.000000000","message":"I believe it does cover missing section and missing option indeed.\n\nIn production, I would strongly recommend against using 0 in max connections. This would dramatically kill any kind of operation on the drive because of too many rsync processes. FYI, in production, we also have a cgroup to slow down rsync processes. Here\u0027s an example:\n\n# cat /etc/systemd/system/rsync.slice \n[Slice]\nIOAccounting\u003d1\nIOWriteBandwidthMax\u003d/srv/node/sda 80M\nIOWriteIOPSMax\u003d/srv/node/sda 100\n\n# cat /etc/systemd/system/rsync.service.d/io-limit-slice.conf \n[Service]\nSlice\u003drsync.slice\nIOSchedulingClass\u003dbest-effort\nIOSchedulingPriority\u003d7\n\nSo I am in the opinion that we should document this as best recommendation, and avoid writing \"max connections \u003d 0\".","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":78,"context_line":"def configure_rsyncd_conf(account_rs, account_mc, account_secname,"},{"line_number":79,"context_line":"                          container_rs, container_mc, container_secname,"},{"line_number":80,"context_line":"                          object_rs, object_mc, object_secname,"},{"line_number":81,"context_line":"                          storage_p, rsyncd_p, logger, sf\u003dNone):"},{"line_number":82,"context_line":"    # Load the rsyncd.conf file, adding"},{"line_number":83,"context_line":"    # a fake global section."},{"line_number":84,"context_line":"    fake_section \u003d \u0027[fake_section_to_please_configobj]\\n\u0027"}],"source_content_type":"text/x-python","patch_set":31,"id":"c993a37b_9664213c","line":81,"range":{"start_line":81,"start_character":55,"end_line":81,"end_character":62},"updated":"2024-03-05 20:20:06.000000000","message":"We never test overriding `source_file`.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"173b7f88c3309eb0175e2e7219356fd4e6608319","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def configure_rsyncd_conf(account_rs, account_mc, account_secname,"},{"line_number":79,"context_line":"                          container_rs, container_mc, container_secname,"},{"line_number":80,"context_line":"                          object_rs, object_mc, object_secname,"},{"line_number":81,"context_line":"                          storage_p, rsyncd_p, logger, sf\u003dNone):"},{"line_number":82,"context_line":"    # Load the rsyncd.conf file, adding"},{"line_number":83,"context_line":"    # a fake global section."},{"line_number":84,"context_line":"    fake_section \u003d \u0027[fake_section_to_please_configobj]\\n\u0027"}],"source_content_type":"text/x-python","patch_set":31,"id":"8187ebdf_217de283","line":81,"range":{"start_line":81,"start_character":55,"end_line":81,"end_character":62},"in_reply_to":"39111a36_4cc9b93e","updated":"2024-03-06 23:02:01.000000000","message":"Indeed. Fixed the test as you suggested: one of the test is using a custom source_file path, while the other keeps sf\u003dNone as per default.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee81ee0ce0bcd07eaef4c1080f7b56e0c28a9588","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def configure_rsyncd_conf(account_rs, account_mc, account_secname,"},{"line_number":79,"context_line":"                          container_rs, container_mc, container_secname,"},{"line_number":80,"context_line":"                          object_rs, object_mc, object_secname,"},{"line_number":81,"context_line":"                          storage_p, rsyncd_p, logger, sf\u003dNone):"},{"line_number":82,"context_line":"    # Load the rsyncd.conf file, adding"},{"line_number":83,"context_line":"    # a fake global section."},{"line_number":84,"context_line":"    fake_section \u003d \u0027[fake_section_to_please_configobj]\\n\u0027"}],"source_content_type":"text/x-python","patch_set":31,"id":"39111a36_4cc9b93e","line":81,"range":{"start_line":81,"start_character":55,"end_line":81,"end_character":62},"in_reply_to":"849f6da0_9ebb4051","updated":"2024-03-06 20:16:53.000000000","message":"It\u0027s more about making sure that the expectations around it are enshrined in tests, so the next time I go refactoring this I don\u0027t accidentally break your workflow 😜","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":78,"context_line":"def configure_rsyncd_conf(account_rs, account_mc, account_secname,"},{"line_number":79,"context_line":"                          container_rs, container_mc, container_secname,"},{"line_number":80,"context_line":"                          object_rs, object_mc, object_secname,"},{"line_number":81,"context_line":"                          storage_p, rsyncd_p, logger, sf\u003dNone):"},{"line_number":82,"context_line":"    # Load the rsyncd.conf file, adding"},{"line_number":83,"context_line":"    # a fake global section."},{"line_number":84,"context_line":"    fake_section \u003d \u0027[fake_section_to_please_configobj]\\n\u0027"}],"source_content_type":"text/x-python","patch_set":31,"id":"849f6da0_9ebb4051","line":81,"range":{"start_line":81,"start_character":55,"end_line":81,"end_character":62},"in_reply_to":"c993a37b_9664213c","updated":"2024-03-06 08:40:05.000000000","message":"Indeed. However, that\u0027s how I run drive-full-checker in production, so I believe that\u0027s safe and working.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    try:"},{"line_number":131,"context_line":"        with open(rsyncd_p, \u0027w\u0027) as f:"},{"line_number":132,"context_line":"            f.write(file_out)"},{"line_number":133,"context_line":"    except Exception as err:"},{"line_number":134,"context_line":"        logger.error(\"Unexpected error {}\".format(err))"},{"line_number":135,"context_line":"        return 1"}],"source_content_type":"text/x-python","patch_set":31,"id":"ea0e1e2f_8c2aaf3a","line":132,"updated":"2024-03-05 20:20:06.000000000","message":"Should we do a write-to-tempfile-then-atomic-rename here? How bad could things go if we only write out a partial config?\n\nShould we maybe compare against `file_content` and only attempt to write if there was a change?","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee81ee0ce0bcd07eaef4c1080f7b56e0c28a9588","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    try:"},{"line_number":131,"context_line":"        with open(rsyncd_p, \u0027w\u0027) as f:"},{"line_number":132,"context_line":"            f.write(file_out)"},{"line_number":133,"context_line":"    except Exception as err:"},{"line_number":134,"context_line":"        logger.error(\"Unexpected error {}\".format(err))"},{"line_number":135,"context_line":"        return 1"}],"source_content_type":"text/x-python","patch_set":31,"id":"9afa63d3_3b2d669a","line":132,"in_reply_to":"7d8a5a8e_3dc1c8e6","updated":"2024-03-06 20:16:53.000000000","message":"Process gets OOM-killed, some other process fills up the drive between our open and write, user is running it manually and has phenomenally bad timing with a ^C, ...\n\nBut yeah, it\u0027s probably fine, especially since the file\u0027s so small. I think I\u0027m just used to doing that sort of thing with rings (which involved more write calls, and where the consequences for a truncated write are pretty bad).\n\nI\u0027m also realizing that \"only write if there was a change\" becomes tricky with the `source_file`/`rsyncd_conf_path` split 😕 Meh, it\u0027s probably OK that we\u0027re constantly re-writing, yeah?\n\nI was also going to ask about needing some signal to restart rsyncd, but I just forgot [how that works](https://download.samba.org/pub/rsync/rsync.1#dopt--daemon):\n\n\u003e The daemon will read the config file (rsyncd.conf) on each connect made by a client and respond to requests accordingly.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    try:"},{"line_number":131,"context_line":"        with open(rsyncd_p, \u0027w\u0027) as f:"},{"line_number":132,"context_line":"            f.write(file_out)"},{"line_number":133,"context_line":"    except Exception as err:"},{"line_number":134,"context_line":"        logger.error(\"Unexpected error {}\".format(err))"},{"line_number":135,"context_line":"        return 1"}],"source_content_type":"text/x-python","patch_set":31,"id":"7d8a5a8e_3dc1c8e6","line":132,"in_reply_to":"ea0e1e2f_8c2aaf3a","updated":"2024-03-06 08:40:05.000000000","message":"In what scenario do you envision that writing to /etc/rsyncd.conf would fail? Normally, the file wont change size, it\u0027s also super small, and probably written by the kernel at once, so it should be quite safe, even if the system partition is full. If we write to a temp file first, there\u0027s a possibility that doing that fails if the partition is full, defeating the purpose of drive-full-checker.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":157,"context_line":"    args \u003d parser.parse_args()"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    # disk-full-checker config file parsing"},{"line_number":160,"context_line":"    c \u003d ConfigParser()"},{"line_number":161,"context_line":"    if not c.read(args.config_file):"},{"line_number":162,"context_line":"        print(\"Unable to read config file %s\" % args.conf_path)"},{"line_number":163,"context_line":"        sys.exit(1)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"06f07ced_b122a2e2","line":161,"range":{"start_line":160,"start_character":4,"end_line":161,"end_character":36},"updated":"2024-03-05 20:20:06.000000000","message":"I get why we want to a `ConfigParser` when we\u0027re hacking on rsyncd.conf, but we might still want to use [Swift\u0027s `readconf`](https://github.com/openstack/swift/blob/2.32.0/swift/common/utils/__init__.py#L2572-L2573) here.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":157,"context_line":"    args \u003d parser.parse_args()"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"    # disk-full-checker config file parsing"},{"line_number":160,"context_line":"    c \u003d ConfigParser()"},{"line_number":161,"context_line":"    if not c.read(args.config_file):"},{"line_number":162,"context_line":"        print(\"Unable to read config file %s\" % args.conf_path)"},{"line_number":163,"context_line":"        sys.exit(1)"},{"line_number":164,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"4f2d5427_3285e6eb","line":161,"range":{"start_line":160,"start_character":4,"end_line":161,"end_character":36},"in_reply_to":"06f07ced_b122a2e2","updated":"2024-03-06 08:40:05.000000000","message":"Done as I incorporated your changes from #911357.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    CONF \u003d dict(c.items(\u0027drive-full-checker\u0027))"},{"line_number":166,"context_line":"    device_dir \u003d CONF.get(\u0027device_dir\u0027, \u0027/srv/node\u0027)"},{"line_number":167,"context_line":"    rsyncd_conf_path \u003d CONF.get(\u0027rsyncd_conf_path\u0027, \u0027/etc/rsyncd.conf\u0027)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    account_max_connections \u003d int(CONF.get(\u0027account_max_connections\u0027, 8))"},{"line_number":170,"context_line":"    account_reserved_space \u003d int(CONF.get(\u0027account_reserved_space\u0027, 100)) * GiB"}],"source_content_type":"text/x-python","patch_set":31,"id":"fbd6610c_624fb127","line":167,"updated":"2024-03-05 20:20:06.000000000","message":"I\u0027m not sure I understand the reason for the split between `source_file` and `rsyncd_conf_path`. They default to the same thing (giving a read-modify-write behavior), but I guess you could specify a separate canonical template to always use as a starting point?\n\nWhy would that only be available via CLI and not config? If I just change `rsyncd_conf_path` in the config, do we really want `source_file` to continue defaulting to `\u0027/etc/rsyncd.conf\u0027`?","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee81ee0ce0bcd07eaef4c1080f7b56e0c28a9588","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    CONF \u003d dict(c.items(\u0027drive-full-checker\u0027))"},{"line_number":166,"context_line":"    device_dir \u003d CONF.get(\u0027device_dir\u0027, \u0027/srv/node\u0027)"},{"line_number":167,"context_line":"    rsyncd_conf_path \u003d CONF.get(\u0027rsyncd_conf_path\u0027, \u0027/etc/rsyncd.conf\u0027)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    account_max_connections \u003d int(CONF.get(\u0027account_max_connections\u0027, 8))"},{"line_number":170,"context_line":"    account_reserved_space \u003d int(CONF.get(\u0027account_reserved_space\u0027, 100)) * GiB"}],"source_content_type":"text/x-python","patch_set":31,"id":"5726fa9b_abbc6912","line":167,"in_reply_to":"9d568e1e_f23cfae0","updated":"2024-03-06 20:16:53.000000000","message":"\u003e The point here, is that we do not want to reimplement the partition checks within puppet, and leave that job to drive-full-checker.\n\nAbsolutely -- which is part of why I feel like handling missing options is important. If we\u0027re dictating `max_connections` from `drive-full-checker.conf`, there\u0027s no reason that we should need to write them to `/etc/swift/rsyncd.conf`. I figure, the fewer places we need to write down config, the easier it\u0027ll be to reason about (and the fewer places we\u0027ll run into conflicts later).","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    CONF \u003d dict(c.items(\u0027drive-full-checker\u0027))"},{"line_number":166,"context_line":"    device_dir \u003d CONF.get(\u0027device_dir\u0027, \u0027/srv/node\u0027)"},{"line_number":167,"context_line":"    rsyncd_conf_path \u003d CONF.get(\u0027rsyncd_conf_path\u0027, \u0027/etc/rsyncd.conf\u0027)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    account_max_connections \u003d int(CONF.get(\u0027account_max_connections\u0027, 8))"},{"line_number":170,"context_line":"    account_reserved_space \u003d int(CONF.get(\u0027account_reserved_space\u0027, 100)) * GiB"}],"source_content_type":"text/x-python","patch_set":31,"id":"9d568e1e_f23cfae0","line":167,"in_reply_to":"fbd6610c_624fb127","updated":"2024-03-06 08:40:05.000000000","message":"Typically, the source_file will be written by your config management tool. What *we* do, is that we use puppet to write /etc/swift/rsyncd.conf, and use that as source_file, while rsyncd_conf_path is written by the drive-full-checker.\n\nThe point here, is that we do not want to reimplement the partition checks within puppet, and leave that job to drive-full-checker. That\u0027s why puppet should not write to /etc/rsyncd.conf directly, and have puppet disagree with drive-full-checker on what should be the value of \"max connections\". Doing this also helps having puppet runs that do not change any value (ie: \"no white lines\" in the puppet run if you know what I mean).\n\nIn my production cluster that has the drive-full-checker, I have \"/var/spool/cron/crontabs/root\" containing: \"*/5 * * * * swift-drive-full-checker --source-file /etc/swift/rsyncd.conf\". I do not mind having the --source-file parameter included in the config file if you think it\u0027s nicer, but I do believe it\u0027s nicer on the command line: typically, one would run without the parameter to check what\u0027s going on with drive-full-checker online modifications, but set the cron job as I did.\n\nPlease let me know if you still think we should go the config file route for this parameter.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b11014fa432ff79f53886b360303d7a74f14f38f","unresolved":true,"context_lines":[{"line_number":180,"context_line":"    object_max_connections \u003d int(CONF.get(\u0027object_max_connections\u0027, 8))"},{"line_number":181,"context_line":"    object_reserved_space \u003d int(CONF.get(\u0027object_reserved_space\u0027, 100)) * GiB"},{"line_number":182,"context_line":"    object_rsyncd_section_name \u003d CONF.get(\u0027object_rsyncd_section_name\u0027,"},{"line_number":183,"context_line":"                                          \u0027 object_{} \u0027)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # logging facility setup"},{"line_number":186,"context_line":"    log_to_console \u003d config_true_value(CONF.get(\u0027log_to_console\u0027, False))"}],"source_content_type":"text/x-python","patch_set":31,"id":"8dacf048_47886389","line":183,"updated":"2024-03-05 20:20:06.000000000","message":"I might be partial towards just passing the config straight in -- then `_patch_rsyncdconf_entry` could take a `server_type` arg to tell it whether to look at account/container/object.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"5211d17c5172be801ae047bd2fe2a43269779930","unresolved":false,"context_lines":[{"line_number":180,"context_line":"    object_max_connections \u003d int(CONF.get(\u0027object_max_connections\u0027, 8))"},{"line_number":181,"context_line":"    object_reserved_space \u003d int(CONF.get(\u0027object_reserved_space\u0027, 100)) * GiB"},{"line_number":182,"context_line":"    object_rsyncd_section_name \u003d CONF.get(\u0027object_rsyncd_section_name\u0027,"},{"line_number":183,"context_line":"                                          \u0027 object_{} \u0027)"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    # logging facility setup"},{"line_number":186,"context_line":"    log_to_console \u003d config_true_value(CONF.get(\u0027log_to_console\u0027, False))"}],"source_content_type":"text/x-python","patch_set":31,"id":"32c156fc_8a03996e","line":183,"in_reply_to":"8dacf048_47886389","updated":"2024-03-06 08:40:05.000000000","message":"Not sure what you mean here.","commit_id":"2987d0e46b090340ed7db75e209698508f6155e9"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ee81ee0ce0bcd07eaef4c1080f7b56e0c28a9588","unresolved":true,"context_lines":[{"line_number":144,"context_line":"                        help\u003d\u0027Path to the drive-full-checker.conf. Default to \u0027"},{"line_number":145,"context_line":"                             \u0027/etc/swift/drive-full-checker.conf\u0027)"},{"line_number":146,"context_line":"    parser.add_argument(\u0027-s\u0027, \u0027--source-file\u0027,"},{"line_number":147,"context_line":"                        default\u003d\u0027/etc/rsyncd.conf\u0027,"},{"line_number":148,"context_line":"                        help\u003d\u0027Path to the source file. Default to \u0027"},{"line_number":149,"context_line":"                             \u0027/etc/rsyncd.conf\u0027)"},{"line_number":150,"context_line":"    args \u003d parser.parse_args()"}],"source_content_type":"text/x-python","patch_set":32,"id":"576cdaf4_84cb7958","line":147,"range":{"start_line":147,"start_character":24,"end_line":147,"end_character":50},"updated":"2024-03-06 20:16:53.000000000","message":"I think this should default to `None` (i.e., whatever the config has for `rsyncd_conf_path`).","commit_id":"5592dad99536408fe2e27133dbfa1aab793bc4f6"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"173b7f88c3309eb0175e2e7219356fd4e6608319","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                        help\u003d\u0027Path to the drive-full-checker.conf. Default to \u0027"},{"line_number":145,"context_line":"                             \u0027/etc/swift/drive-full-checker.conf\u0027)"},{"line_number":146,"context_line":"    parser.add_argument(\u0027-s\u0027, \u0027--source-file\u0027,"},{"line_number":147,"context_line":"                        default\u003d\u0027/etc/rsyncd.conf\u0027,"},{"line_number":148,"context_line":"                        help\u003d\u0027Path to the source file. Default to \u0027"},{"line_number":149,"context_line":"                             \u0027/etc/rsyncd.conf\u0027)"},{"line_number":150,"context_line":"    args \u003d parser.parse_args()"}],"source_content_type":"text/x-python","patch_set":32,"id":"069c0aac_fd78ebba","line":147,"range":{"start_line":147,"start_character":24,"end_line":147,"end_character":50},"in_reply_to":"576cdaf4_84cb7958","updated":"2024-03-06 23:02:01.000000000","message":"Right. So that if one changes rsyncd_conf_path is changed, the source follows. Did what you suggested in my last patch.","commit_id":"5592dad99536408fe2e27133dbfa1aab793bc4f6"}]}