)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"a004d66f_c1991daf","updated":"2026-10-23 15:10:05.000000000","message":"I probably have enough comments here to warrant the -1","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"698213d7_e234fb0f","updated":"2026-10-28 16:42:15.000000000","message":"I\u0027ve only looked at the differences between PS6 and PS8, given my thorough review of PS6 previously. All my points appear to have been addressed.","commit_id":"615ede8b66a67742b5db6c794a6ed0bc61e0a441"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"62c978aef8f926d5a83440751a300e31f68cc58f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9355fd77_63579769","updated":"2026-10-29 07:12:09.000000000","message":"and had to reset your +2 here too :/\n\nchanged one thing in _pass_check():\n\n- Every ConsulException from a check pass was treated as a lost check.\n- So behind a reverse proxy that answers 502 (or when Consul answers 403 or 429), each heartbeat tried to register every member again. Those restores failed too, doubled the requests, and logged a misleading \"Failed to restore\" warning.\n\nNow only Consul\u0027s \"unknown check\" answers restore the registration: 500 on Consul 1.10, 404 on later versions. Any other error logs \"Failed to pass the Consul check of \u003cservice\u003e\" and is retried at the next heartbeat.\n\nheartbeat() still returns normally in that case, as before. Raising instead would stop a caller that heartbeats several coordinators at the first failure, so its healthy members would expire. The new test covers 502, 503, 504, 403, 429 and 400, plus a 500 that still restores.\n\nNothing else changed apart from the rebase onto the new 1007613.\n\nThanks Claude :D the level of testing I\u0027m able to do now is kind of insane compared to what I would have done before TBH 🤯","commit_id":"b89687457cecaac00737a18c7307b066cd925329"}],"doc/source/user/drivers.rst":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":305,"context_line":"  }"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"With another namespace or service name template, change the prefixes to"},{"line_number":308,"context_line":"match: with ``{group}``, for example, the service names are the group names."},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"Kubernetes"},{"line_number":311,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"eb37a995_a37f9feb","line":308,"updated":"2026-10-23 15:10:05.000000000","message":"This doc is starting to get quite lengthy. We probably want to split `drivers.rst` into `drivers/index.rst`, `drivers/consul.rst` etc. One for a follow-up though.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":305,"context_line":"  }"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"With another namespace or service name template, change the prefixes to"},{"line_number":308,"context_line":"match: with ``{group}``, for example, the service names are the group names."},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"Kubernetes"},{"line_number":311,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"380c63ea_ee1a9ab9","line":308,"in_reply_to":"559a2235_ba0454c5","updated":"2026-10-28 16:42:15.000000000","message":"Sorry, I should have been clearer here: this was me musing. A separate follow-up is fine 👍","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":305,"context_line":"  }"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"With another namespace or service name template, change the prefixes to"},{"line_number":308,"context_line":"match: with ``{group}``, for example, the service names are the group names."},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"Kubernetes"},{"line_number":311,"context_line":"----------"}],"source_content_type":"text/x-rst","patch_set":6,"id":"559a2235_ba0454c5","line":308,"in_reply_to":"eb37a995_a37f9feb","updated":"2026-10-23 19:27:33.000000000","message":"Happy to tackle the doc split either in this CR or as a separate follow-up, whichever you\u0027d prefer, just lmk","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"}],"tooz/drivers/consul.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":367,"context_line":"        except (AttributeError, KeyError, IndexError, ValueError) as e:"},{"line_number":368,"context_line":"            raise ValueError(\"Invalid service_name_template\") from e"},{"line_number":369,"context_line":"        if \"{group}\" not in self._service_name_template:"},{"line_number":370,"context_line":"            raise ValueError(\"service_name_template must contain {group}\")"},{"line_number":371,"context_line":"        # \"ttl\" is the name used before this driver took the name the other"},{"line_number":372,"context_line":"        # drivers use."},{"line_number":373,"context_line":"        timeout \u003d opts.get(\"timeout\", 15)"},{"line_number":374,"context_line":"        self._ttl \u003d float("},{"line_number":375,"context_line":"            opts.get(\"membership_timeout\", opts.get(\"ttl\", timeout))"}],"source_content_type":"text/x-python","patch_set":6,"id":"5184744a_3751f0dd","line":372,"range":{"start_line":370,"start_character":74,"end_line":372,"end_character":22},"updated":"2026-10-23 15:10:05.000000000","message":"But this is a new driver: why was it using `ttl` before? 😕 Was it? What am I missing?\n\nThis comment is also detached from the line in question, I think (it should be down a line)","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":367,"context_line":"        except (AttributeError, KeyError, IndexError, ValueError) as e:"},{"line_number":368,"context_line":"            raise ValueError(\"Invalid service_name_template\") from e"},{"line_number":369,"context_line":"        if \"{group}\" not in self._service_name_template:"},{"line_number":370,"context_line":"            raise ValueError(\"service_name_template must contain {group}\")"},{"line_number":371,"context_line":"        # \"ttl\" is the name used before this driver took the name the other"},{"line_number":372,"context_line":"        # drivers use."},{"line_number":373,"context_line":"        timeout \u003d opts.get(\"timeout\", 15)"},{"line_number":374,"context_line":"        self._ttl \u003d float("},{"line_number":375,"context_line":"            opts.get(\"membership_timeout\", opts.get(\"ttl\", timeout))"}],"source_content_type":"text/x-python","patch_set":6,"id":"df05187a_e56b04ef","line":372,"range":{"start_line":370,"start_character":74,"end_line":372,"end_character":22},"in_reply_to":"5184744a_3751f0dd","updated":"2026-10-23 19:27:33.000000000","message":"You are right: this referred to an earlier, unmerged version of the Consul driver. I removed the ttl alias and the misleading comment, since our RPC v1 migration does not need it.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":367,"context_line":"        except (AttributeError, KeyError, IndexError, ValueError) as e:"},{"line_number":368,"context_line":"            raise ValueError(\"Invalid service_name_template\") from e"},{"line_number":369,"context_line":"        if \"{group}\" not in self._service_name_template:"},{"line_number":370,"context_line":"            raise ValueError(\"service_name_template must contain {group}\")"},{"line_number":371,"context_line":"        # \"ttl\" is the name used before this driver took the name the other"},{"line_number":372,"context_line":"        # drivers use."},{"line_number":373,"context_line":"        timeout \u003d opts.get(\"timeout\", 15)"},{"line_number":374,"context_line":"        self._ttl \u003d float("},{"line_number":375,"context_line":"            opts.get(\"membership_timeout\", opts.get(\"ttl\", timeout))"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ed1739e_3751cf0e","line":372,"range":{"start_line":370,"start_character":74,"end_line":372,"end_character":22},"in_reply_to":"df05187a_e56b04ef","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":383,"context_line":"        if self._ttl \u003c\u003d 0:"},{"line_number":384,"context_line":"            raise ValueError(\"membership_timeout must be greater than zero\")"},{"line_number":385,"context_line":"        self._request_timeout \u003d float("},{"line_number":386,"context_line":"            opts.get(\"request_timeout\", min(5.0, self._ttl / 3))"},{"line_number":387,"context_line":"        )"},{"line_number":388,"context_line":"        if self._request_timeout \u003c\u003d 0:"},{"line_number":389,"context_line":"            raise ValueError(\"request_timeout must be greater than zero\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"d511edc1_2e01ac60","line":386,"updated":"2026-10-23 15:10:05.000000000","message":"why 3?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        if self._ttl \u003c\u003d 0:"},{"line_number":384,"context_line":"            raise ValueError(\"membership_timeout must be greater than zero\")"},{"line_number":385,"context_line":"        self._request_timeout \u003d float("},{"line_number":386,"context_line":"            opts.get(\"request_timeout\", min(5.0, self._ttl / 3))"},{"line_number":387,"context_line":"        )"},{"line_number":388,"context_line":"        if self._request_timeout \u003c\u003d 0:"},{"line_number":389,"context_line":"            raise ValueError(\"request_timeout must be greater than zero\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"5ac18e27_29b65558","line":386,"in_reply_to":"3d10a159_c56e4c42","updated":"2026-10-28 16:42:15.000000000","message":"Once there\u0027s *some* (documented) logic behind the magic number I\u0027m happy.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":383,"context_line":"        if self._ttl \u003c\u003d 0:"},{"line_number":384,"context_line":"            raise ValueError(\"membership_timeout must be greater than zero\")"},{"line_number":385,"context_line":"        self._request_timeout \u003d float("},{"line_number":386,"context_line":"            opts.get(\"request_timeout\", min(5.0, self._ttl / 3))"},{"line_number":387,"context_line":"        )"},{"line_number":388,"context_line":"        if self._request_timeout \u003c\u003d 0:"},{"line_number":389,"context_line":"            raise ValueError(\"request_timeout must be greater than zero\")"}],"source_content_type":"text/x-python","patch_set":6,"id":"3d10a159_c56e4c42","line":386,"in_reply_to":"d511edc1_2e01ac60","updated":"2026-10-23 19:27:33.000000000","message":"It is a conservative per-request budget, capped at five seconds, to leave some time before membership expiry if an agent stalls. Three is a chosen margin, not a Consul requirement or a bound on the whole heartbeat pass; I added a short comment to make that distinction clear. It seemed to work well in our testing, I guess I could also maybe figure out a way to make it configurable or something?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":387,"context_line":"        )"},{"line_number":388,"context_line":"        if self._request_timeout \u003c\u003d 0:"},{"line_number":389,"context_line":"            raise ValueError(\"request_timeout must be greater than zero\")"},{"line_number":390,"context_line":"        self._deregister_after \u003d int(opts.get(\"deregister_after\", 600))"},{"line_number":391,"context_line":"        tls_skip_verify \u003d strutils.bool_from_string("},{"line_number":392,"context_line":"            opts.get(\"tls_skip_verify\", False)"},{"line_number":393,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":6,"id":"9321362f_944c925e","line":390,"updated":"2026-10-23 15:10:05.000000000","message":"do you want a minimum here?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":387,"context_line":"        )"},{"line_number":388,"context_line":"        if self._request_timeout \u003c\u003d 0:"},{"line_number":389,"context_line":"            raise ValueError(\"request_timeout must be greater than zero\")"},{"line_number":390,"context_line":"        self._deregister_after \u003d int(opts.get(\"deregister_after\", 600))"},{"line_number":391,"context_line":"        tls_skip_verify \u003d strutils.bool_from_string("},{"line_number":392,"context_line":"            opts.get(\"tls_skip_verify\", False)"},{"line_number":393,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":6,"id":"feabd1d4_6b18bc77","line":390,"in_reply_to":"90cfab3f_9fe24760","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":387,"context_line":"        )"},{"line_number":388,"context_line":"        if self._request_timeout \u003c\u003d 0:"},{"line_number":389,"context_line":"            raise ValueError(\"request_timeout must be greater than zero\")"},{"line_number":390,"context_line":"        self._deregister_after \u003d int(opts.get(\"deregister_after\", 600))"},{"line_number":391,"context_line":"        tls_skip_verify \u003d strutils.bool_from_string("},{"line_number":392,"context_line":"            opts.get(\"tls_skip_verify\", False)"},{"line_number":393,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":6,"id":"90cfab3f_9fe24760","line":390,"in_reply_to":"9321362f_944c925e","updated":"2026-10-23 19:27:33.000000000","message":"Added validation rejecting negative values and clarified the docs. Zero intentionally disables automatic deregistration; Consul clamps positive values below its minimum (normally 60 seconds). Added coverage for negative, zero, and positive values so a local minimum cannot accidentally change the zero case.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":506,"context_line":"                raise"},{"line_number":507,"context_line":"            except (requests.RequestException, OSError) as e:"},{"line_number":508,"context_line":"                # Pass the other members\u0027 checks before failing."},{"line_number":509,"context_line":"                error \u003d error or e"},{"line_number":510,"context_line":"        for lock in list(self._held_locks):"},{"line_number":511,"context_line":"            lock.heartbeat()"},{"line_number":512,"context_line":"        if error is not None:"},{"line_number":513,"context_line":"            raise error"},{"line_number":514,"context_line":"        return min(self._ttl, self._lock_ttl) / 2"}],"source_content_type":"text/x-python","patch_set":6,"id":"4fd23a55_488f6e7a","line":511,"range":{"start_line":509,"start_character":34,"end_line":511,"end_character":28},"updated":"2026-10-23 15:10:05.000000000","message":"nit: why not do this first, before checking the registrations? That would allow the error handling to happen immediately after the error checking.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":506,"context_line":"                raise"},{"line_number":507,"context_line":"            except (requests.RequestException, OSError) as e:"},{"line_number":508,"context_line":"                # Pass the other members\u0027 checks before failing."},{"line_number":509,"context_line":"                error \u003d error or e"},{"line_number":510,"context_line":"        for lock in list(self._held_locks):"},{"line_number":511,"context_line":"            lock.heartbeat()"},{"line_number":512,"context_line":"        if error is not None:"},{"line_number":513,"context_line":"            raise error"},{"line_number":514,"context_line":"        return min(self._ttl, self._lock_ttl) / 2"}],"source_content_type":"text/x-python","patch_set":6,"id":"96a52aa1_9dcafc7a","line":511,"range":{"start_line":509,"start_character":34,"end_line":511,"end_character":28},"in_reply_to":"4fd23a55_488f6e7a","updated":"2026-10-23 19:27:33.000000000","message":"Done. Locks are renewed first, with a regression test proving that a membership request timing out cannot skip their renewal.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                raise"},{"line_number":507,"context_line":"            except (requests.RequestException, OSError) as e:"},{"line_number":508,"context_line":"                # Pass the other members\u0027 checks before failing."},{"line_number":509,"context_line":"                error \u003d error or e"},{"line_number":510,"context_line":"        for lock in list(self._held_locks):"},{"line_number":511,"context_line":"            lock.heartbeat()"},{"line_number":512,"context_line":"        if error is not None:"},{"line_number":513,"context_line":"            raise error"},{"line_number":514,"context_line":"        return min(self._ttl, self._lock_ttl) / 2"}],"source_content_type":"text/x-python","patch_set":6,"id":"b77899c2_14c22f76","line":511,"range":{"start_line":509,"start_character":34,"end_line":511,"end_character":28},"in_reply_to":"96a52aa1_9dcafc7a","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":513,"context_line":"            raise error"},{"line_number":514,"context_line":"        return min(self._ttl, self._lock_ttl) / 2"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"    def _pass_check(self, registration: _Registration) -\u003e None:"},{"line_number":517,"context_line":"        try:"},{"line_number":518,"context_line":"            passed \u003d self._client.agent.check.ttl_pass(registration.check_id)"},{"line_number":519,"context_line":"        except consul.ConsulException:"}],"source_content_type":"text/x-python","patch_set":6,"id":"81329b10_31f3dda9","line":516,"updated":"2026-10-23 15:10:05.000000000","message":"Personally,unless we plan to use this elsewhere I would inline this into `heartbeat`, e.g. replacing the `return`s with `continue`s. The resulting function won\u0027t be huge and its purpose would be clearer. IMO ofc.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":513,"context_line":"            raise error"},{"line_number":514,"context_line":"        return min(self._ttl, self._lock_ttl) / 2"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"    def _pass_check(self, registration: _Registration) -\u003e None:"},{"line_number":517,"context_line":"        try:"},{"line_number":518,"context_line":"            passed \u003d self._client.agent.check.ttl_pass(registration.check_id)"},{"line_number":519,"context_line":"        except consul.ConsulException:"}],"source_content_type":"text/x-python","patch_set":6,"id":"b4d954f4_9633a288","line":516,"in_reply_to":"2d22dc10_d4caa69a","updated":"2026-10-28 16:42:15.000000000","message":"Okay, if you insist 🙂","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":513,"context_line":"            raise error"},{"line_number":514,"context_line":"        return min(self._ttl, self._lock_ttl) / 2"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":"    def _pass_check(self, registration: _Registration) -\u003e None:"},{"line_number":517,"context_line":"        try:"},{"line_number":518,"context_line":"            passed \u003d self._client.agent.check.ttl_pass(registration.check_id)"},{"line_number":519,"context_line":"        except consul.ConsulException:"}],"source_content_type":"text/x-python","patch_set":6,"id":"2d22dc10_d4caa69a","line":516,"in_reply_to":"81329b10_31f3dda9","updated":"2026-10-23 19:27:33.000000000","message":"I would prefer to keep this helper: it contains the renewal and repair of one registration, including the locking and stale-snapshot check. heartbeat then owns iteration, lock renewal, and the continue-or-abort error policy. Keeping those separate makes the race protection easier to follow.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":516,"context_line":"    def _pass_check(self, registration: _Registration) -\u003e None:"},{"line_number":517,"context_line":"        try:"},{"line_number":518,"context_line":"            passed \u003d self._client.agent.check.ttl_pass(registration.check_id)"},{"line_number":519,"context_line":"        except consul.ConsulException:"},{"line_number":520,"context_line":"            # Consul 1.10 answers 500 for an unknown check, later versions"},{"line_number":521,"context_line":"            # 404, which py-consul reports as False."},{"line_number":522,"context_line":"            passed \u003d False"},{"line_number":523,"context_line":"        if passed:"},{"line_number":524,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"35e93186_6bc8bc3c","line":521,"range":{"start_line":519,"start_character":38,"end_line":521,"end_character":52},"updated":"2026-10-23 15:10:05.000000000","message":"Is this relevant? We don\u0027t see to be checking errors codes?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":516,"context_line":"    def _pass_check(self, registration: _Registration) -\u003e None:"},{"line_number":517,"context_line":"        try:"},{"line_number":518,"context_line":"            passed \u003d self._client.agent.check.ttl_pass(registration.check_id)"},{"line_number":519,"context_line":"        except consul.ConsulException:"},{"line_number":520,"context_line":"            # Consul 1.10 answers 500 for an unknown check, later versions"},{"line_number":521,"context_line":"            # 404, which py-consul reports as False."},{"line_number":522,"context_line":"            passed \u003d False"},{"line_number":523,"context_line":"        if passed:"},{"line_number":524,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"bae95fa5_7a02a158","line":521,"range":{"start_line":519,"start_character":38,"end_line":521,"end_character":52},"in_reply_to":"35e93186_6bc8bc3c","updated":"2026-10-23 19:27:33.000000000","message":"The distinction is in py-consul: the older unknown-check response raises ConsulException, whereas the later 404 response is returned as False. Both need to reach the restore path. I reworded the comment to explain that rather than imply we branch on HTTP status here; tests cover both forms.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":516,"context_line":"    def _pass_check(self, registration: _Registration) -\u003e None:"},{"line_number":517,"context_line":"        try:"},{"line_number":518,"context_line":"            passed \u003d self._client.agent.check.ttl_pass(registration.check_id)"},{"line_number":519,"context_line":"        except consul.ConsulException:"},{"line_number":520,"context_line":"            # Consul 1.10 answers 500 for an unknown check, later versions"},{"line_number":521,"context_line":"            # 404, which py-consul reports as False."},{"line_number":522,"context_line":"            passed \u003d False"},{"line_number":523,"context_line":"        if passed:"},{"line_number":524,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":6,"id":"e840f2ba_54de7a40","line":521,"range":{"start_line":519,"start_character":38,"end_line":521,"end_character":52},"in_reply_to":"bae95fa5_7a02a158","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":522,"context_line":"            passed \u003d False"},{"line_number":523,"context_line":"        if passed:"},{"line_number":524,"context_line":"            return"},{"line_number":525,"context_line":"        with self._ops_lock:"},{"line_number":526,"context_line":"            with self._lock:"},{"line_number":527,"context_line":"                current \u003d self._registrations.get(registration.group_id)"},{"line_number":528,"context_line":"            if current !\u003d registration:"},{"line_number":529,"context_line":"                return"},{"line_number":530,"context_line":"            # The agent lost the registration, for example after a restart"},{"line_number":531,"context_line":"            # without its data directory. Register it again; if Consul"},{"line_number":532,"context_line":"            # refuses, try again at the next heartbeat. Connection errors"}],"source_content_type":"text/x-python","patch_set":6,"id":"30df2a07_8bb11e6c","line":529,"range":{"start_line":525,"start_character":28,"end_line":529,"end_character":22},"updated":"2026-10-23 15:10:05.000000000","message":"It\u0027s not entirely clear to me why these might get out of sync. Could you add a comment?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":522,"context_line":"            passed \u003d False"},{"line_number":523,"context_line":"        if passed:"},{"line_number":524,"context_line":"            return"},{"line_number":525,"context_line":"        with self._ops_lock:"},{"line_number":526,"context_line":"            with self._lock:"},{"line_number":527,"context_line":"                current \u003d self._registrations.get(registration.group_id)"},{"line_number":528,"context_line":"            if current !\u003d registration:"},{"line_number":529,"context_line":"                return"},{"line_number":530,"context_line":"            # The agent lost the registration, for example after a restart"},{"line_number":531,"context_line":"            # without its data directory. Register it again; if Consul"},{"line_number":532,"context_line":"            # refuses, try again at the next heartbeat. Connection errors"}],"source_content_type":"text/x-python","patch_set":6,"id":"d1a4e4d3_d3447fb6","line":529,"range":{"start_line":525,"start_character":28,"end_line":529,"end_character":22},"in_reply_to":"30df2a07_8bb11e6c","updated":"2026-10-23 19:27:33.000000000","message":"Added a comment and a deterministic regression test for both cases: a leave or capability update can happen while ttl_pass is doing I/O, after we took the snapshot. This check prevents restoring a departed member or overwriting its newer capabilities.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":522,"context_line":"            passed \u003d False"},{"line_number":523,"context_line":"        if passed:"},{"line_number":524,"context_line":"            return"},{"line_number":525,"context_line":"        with self._ops_lock:"},{"line_number":526,"context_line":"            with self._lock:"},{"line_number":527,"context_line":"                current \u003d self._registrations.get(registration.group_id)"},{"line_number":528,"context_line":"            if current !\u003d registration:"},{"line_number":529,"context_line":"                return"},{"line_number":530,"context_line":"            # The agent lost the registration, for example after a restart"},{"line_number":531,"context_line":"            # without its data directory. Register it again; if Consul"},{"line_number":532,"context_line":"            # refuses, try again at the next heartbeat. Connection errors"}],"source_content_type":"text/x-python","patch_set":6,"id":"a6db037a_83957172","line":529,"range":{"start_line":525,"start_character":28,"end_line":529,"end_character":22},"in_reply_to":"d1a4e4d3_d3447fb6","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":528,"context_line":"            if current !\u003d registration:"},{"line_number":529,"context_line":"                return"},{"line_number":530,"context_line":"            # The agent lost the registration, for example after a restart"},{"line_number":531,"context_line":"            # without its data directory. Register it again; if Consul"},{"line_number":532,"context_line":"            # refuses, try again at the next heartbeat. Connection errors"},{"line_number":533,"context_line":"            # still fail the heartbeat."},{"line_number":534,"context_line":"            try:"},{"line_number":535,"context_line":"                self._client.agent.service.register("},{"line_number":536,"context_line":"                    registration.name, **registration.kwargs"}],"source_content_type":"text/x-python","patch_set":6,"id":"91666d79_944700e2","line":533,"range":{"start_line":531,"start_character":70,"end_line":533,"end_character":39},"updated":"2026-10-23 15:10:05.000000000","message":"Do we want to explicitly catch and reraisethe connection error with some context logging?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":528,"context_line":"            if current !\u003d registration:"},{"line_number":529,"context_line":"                return"},{"line_number":530,"context_line":"            # The agent lost the registration, for example after a restart"},{"line_number":531,"context_line":"            # without its data directory. Register it again; if Consul"},{"line_number":532,"context_line":"            # refuses, try again at the next heartbeat. Connection errors"},{"line_number":533,"context_line":"            # still fail the heartbeat."},{"line_number":534,"context_line":"            try:"},{"line_number":535,"context_line":"                self._client.agent.service.register("},{"line_number":536,"context_line":"                    registration.name, **registration.kwargs"}],"source_content_type":"text/x-python","patch_set":6,"id":"d713643b_794adb6c","line":533,"range":{"start_line":531,"start_character":70,"end_line":533,"end_character":39},"in_reply_to":"91666d79_944700e2","updated":"2026-10-23 19:27:33.000000000","message":"Connection errors already propagate through heartbeat and are translated to ToozConnectionError. I added service-ID context at that boundary at DEBUG level, preserving the existing retry/error handling and avoiding another warning on every retry.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":528,"context_line":"            if current !\u003d registration:"},{"line_number":529,"context_line":"                return"},{"line_number":530,"context_line":"            # The agent lost the registration, for example after a restart"},{"line_number":531,"context_line":"            # without its data directory. Register it again; if Consul"},{"line_number":532,"context_line":"            # refuses, try again at the next heartbeat. Connection errors"},{"line_number":533,"context_line":"            # still fail the heartbeat."},{"line_number":534,"context_line":"            try:"},{"line_number":535,"context_line":"                self._client.agent.service.register("},{"line_number":536,"context_line":"                    registration.name, **registration.kwargs"}],"source_content_type":"text/x-python","patch_set":6,"id":"f043fc5a_239d2c15","line":533,"range":{"start_line":531,"start_character":70,"end_line":533,"end_character":39},"in_reply_to":"d713643b_794adb6c","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"981c43c9e16d1c70320088bae8aaf383a6137c94","unresolved":true,"context_lines":[{"line_number":597,"context_line":"        # Once per service, not at every read."},{"line_number":598,"context_line":"        service_id \u003d entry[\"Service\"].get(\"ID\")"},{"line_number":599,"context_line":"        if service_id not in self._malformed:"},{"line_number":600,"context_line":"            if len(self._malformed) \u003e\u003d 1024:"},{"line_number":601,"context_line":"                self._malformed.clear()"},{"line_number":602,"context_line":"            self._malformed.add(service_id)"},{"line_number":603,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"a202fe8d_fd2c1197","line":600,"updated":"2026-10-23 15:10:05.000000000","message":"This is here to stop the cache growing without bound, I assume? Want to call that out?","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48656d0797c806ed4d36377fb8d5f4fdc7a5271a","unresolved":false,"context_lines":[{"line_number":597,"context_line":"        # Once per service, not at every read."},{"line_number":598,"context_line":"        service_id \u003d entry[\"Service\"].get(\"ID\")"},{"line_number":599,"context_line":"        if service_id not in self._malformed:"},{"line_number":600,"context_line":"            if len(self._malformed) \u003e\u003d 1024:"},{"line_number":601,"context_line":"                self._malformed.clear()"},{"line_number":602,"context_line":"            self._malformed.add(service_id)"},{"line_number":603,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"c764cdf4_b54df9ec","line":600,"in_reply_to":"8b8bc888_036d9e28","updated":"2026-10-28 16:42:15.000000000","message":"Acknowledged","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6e6f8bfb97f06dddce1b31d0f0fe146e465f0155","unresolved":true,"context_lines":[{"line_number":597,"context_line":"        # Once per service, not at every read."},{"line_number":598,"context_line":"        service_id \u003d entry[\"Service\"].get(\"ID\")"},{"line_number":599,"context_line":"        if service_id not in self._malformed:"},{"line_number":600,"context_line":"            if len(self._malformed) \u003e\u003d 1024:"},{"line_number":601,"context_line":"                self._malformed.clear()"},{"line_number":602,"context_line":"            self._malformed.add(service_id)"},{"line_number":603,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":6,"id":"8b8bc888_036d9e28","line":600,"in_reply_to":"a202fe8d_fd2c1197","updated":"2026-10-23 19:27:33.000000000","message":"Correct, I added a comment explaining that it bounds warning-suppression state when malformed registrations keep arriving under new service IDs. Also removed the overly absolute “once per service” wording, since clearing the set allows a warning to recur.","commit_id":"c6090f7c3b1e5a6b28785b148ffa483f0d6c33db"}]}
