)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5366b7f7_ef3d5b9e","updated":"2026-09-10 03:17:33.000000000","message":"Initial review by GPT 6 Astra with context of the full patch chain, I reviewed the review myself and all of this seems valid, but I think only about half of it applies to your specific case, and no one else actually uses the old version I believe so I doubt we\u0027ll ever need to merge this, or account for anything outside of your very narrow deployment case. You can deploy with this patch, then actually remove the entire patch and be fine, so I don\u0027t expect this one will ever need to merge?","commit_id":"badc646943b220279a3c4610da90126e34974703"},{"author":{"_account_id":33451,"name":"Yushiro Furukawa","display_name":"Yushiro Furukawa","email":"yushiro.furukawa@lycorp.co.jp","username":"yushiro2"},"change_message_id":"6a2aa2512a7125328b5929d857acab0837aaa590","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7b9fd4ce_b15f14e4","updated":"2026-09-14 15:09:04.000000000","message":"Thanks Adam for your comment.  After discussed with him, we decided to combine these comments into existing PS.  So, I\u0027ll abandon this patch.","commit_id":"badc646943b220279a3c4610da90126e34974703"}],"doc/source/admin/http-driver.rst":[{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":true,"context_lines":[{"line_number":174,"context_line":"    ssl_require_client_cert \u003d false"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"Upgrade the broadcaster and service processes, verify that no legacy catalog"},{"line_number":177,"context_line":"entries remain, then disable the option everywhere. Legacy calls retain the"},{"line_number":178,"context_line":"old synchronous timeout behavior and do not use HTTP 202 polling; native peers"},{"line_number":179,"context_line":"continue to use HTTP/2 and long-call polling throughout the migration."},{"line_number":180,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"6e6069b2_576d6298","line":177,"updated":"2026-09-10 03:17:33.000000000","message":"[P2] Define a migration completion condition that can actually be reached\n\nThis completion condition is circular: while compatibility is enabled, every upgraded listener deliberately creates a legacy alias. Therefore legacy catalog entries remain even after the last old process has been upgraded. Waiting for the old catalog to become empty before disabling the option is not a usable cutover procedure.\n\nPlease distinguish legacy-only processes/endpoints from aliases owned by upgraded listeners. Document how the operator verifies that no old producers, listeners, or broadcasters remain, allows cached legacy endpoint selections and in-flight legacy calls/fanout work to drain, then disables compatibility and removes its aliases. Checking only the catalog cannot establish that old producer-only processes are gone. State the exact legacy revision and deployment assumptions used to validate the procedure, and describe a safe rollback point.\n\nValidate the sequence with a real mixed-version rollout, including a long-running request across cutover. After the last old process exits, the completion check must succeed even though upgraded listeners still have aliases. Then disable/remove the bridge and verify native discovery and calls/casts/fanout still work and obsolete aliases are removed. Re-enabling the bridge for rollback should restore its registration behavior.\n\nThis matters even for a temporary downstream bridge: correct wire translation alone does not demonstrate a safe zero-downtime cutover.","commit_id":"badc646943b220279a3c4610da90126e34974703"}],"oslo_messaging/_drivers/http_driver/tooz_broker.py":[{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":true,"context_lines":[{"line_number":169,"context_line":"        cert_cert \u003d option(\u0027cert_cert\u0027)"},{"line_number":170,"context_line":"        cert_key \u003d option(\u0027cert_key\u0027)"},{"line_number":171,"context_line":"        cert \u003d (cert_cert, cert_key) if cert_cert and cert_key else None"},{"line_number":172,"context_line":"        self.client \u003d consul.Consul("},{"line_number":173,"context_line":"            host\u003dparsed.hostname or consul_conf.host,"},{"line_number":174,"context_line":"            port\u003dparsed.port or consul_conf.port,"},{"line_number":175,"context_line":"            token\u003doption(\u0027acl_token\u0027, consul_conf.token),"}],"source_content_type":"text/x-python","patch_set":1,"id":"3e1fac43_6b0813c4","line":172,"updated":"2026-09-10 03:17:33.000000000","message":"[P1] Bound all requests made by the separate legacy Consul client\n\nThis new client bypasses the bounded Consul HTTP adapter used by TooZ. In the tested py-consul 1.7.1, the default Requests client does not supply a request timeout, and this constructor does not read the URL\u0027s request_timeout option. health_check_timeout below controls Consul\u0027s endpoint probe; it does not bound this process\u0027s requests to the Consul API.\n\nReproduced with request_timeout\u003d0.05 and a local agent endpoint that delays its response for 0.5 seconds: the legacy request returns successfully after about 0.502 seconds. This is a bounded reproduction of the missing timeout, not a test that was left hung indefinitely. A genuinely stalled lookup can hold the discovery refresh worker/lock; registration and shutdown deregistration can block too. The outer RPC deadline does not cancel that backend request.\n\nPlease apply a validated finite request timeout, consistent with TooZ\u0027s option semantics, to every legacy API operation, and own/close the HTTP session explicitly. Preserve the configured token, CA verification, and client certificate settings.\n\nRegression coverage: delay a real local Consul API response and verify discovery and registration/deregistration fail within a bounded time; verify resources close on shutdown and an unavailable legacy catalog cannot indefinitely block native discovery or cleanup.","commit_id":"badc646943b220279a3c4610da90126e34974703"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":true,"context_lines":[{"line_number":279,"context_line":"            reverse_proxy_endpoint\u003dreverse_proxy,"},{"line_number":280,"context_line":"            rpchost\u003dhost,"},{"line_number":281,"context_line":"            rpchost_port\u003dport,"},{"line_number":282,"context_line":"            tls_server_name\u003d(tags[0] if tags else host),"},{"line_number":283,"context_line":"            server_hostname\u003d(tags[0] if tags else None),"},{"line_number":284,"context_line":"            legacy_http_rpc\u003dTrue)"},{"line_number":285,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"fcda100e_a00406e7","line":282,"updated":"2026-09-10 03:17:33.000000000","message":"[P1] Preserve the legacy address as the TLS verification identity\n\nPlease use the legacy catalog Address for direct-connection TLS verification, rather than Tags[0]. The tag is the logical RPC server name; it is not necessarily a certificate identity. Keep it in server_hostname for directed discovery.\n\nReproduced on PS1 with a real Hypercorn HTTPS listener, a trusted certificate whose IP SAN is 127.0.0.1, and a legacy entry with Address\u003d127.0.0.1 and Tags\u003d[\u0027compute-1\u0027]. The actual legacy HTTPClient from 973843 PS14 succeeds. The compatibility client fails certificate verification because this value overrides the URL identity with compute-1. Changing only the Service TLS name to Address makes the same request succeed.\n\nSuggested fix: set the direct legacy TLS identity to host while leaving server_hostname based on the tag. Preserve the existing rule that a reverse proxy is verified against the proxy\u0027s own identity. Do not disable certificate verification.\n\nRegression coverage: use a trusted local HTTPS endpoint with different address and RPC tag, assert that old and compatibility clients both succeed, and retain a negative certificate-name mismatch test plus directed-routing coverage. This affects verified HTTPS deployments; plain HTTP does not exercise it.","commit_id":"badc646943b220279a3c4610da90126e34974703"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":true,"context_lines":[{"line_number":813,"context_line":"                self._legacy_catalog.register("},{"line_number":814,"context_line":"                    name, service_id, host, port, tags,"},{"line_number":815,"context_line":"                    reverse_proxy_endpoint, enable_ssl)"},{"line_number":816,"context_line":"                self._legacy_service_ids.add(service_id)"},{"line_number":817,"context_line":"            except Exception:"},{"line_number":818,"context_line":"                LOG.exception("},{"line_number":819,"context_line":"                    \u0027Failed to register legacy Consul alias %s for %s\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"8c764ab3_46dc4025","line":816,"updated":"2026-09-10 03:17:33.000000000","message":"[P1] Reconcile legacy aliases after registration loss\n\nThe legacy alias is registered once, but only its ID is retained. Native TooZ membership has recovery; that recovery does not restore this separate registration. Consul can remove the alias after a sustained critical check, or registration state can otherwise be lost. Old processes then stop discovering a healthy upgraded listener until it is explicitly re-registered.\n\nReproduced on PS1 against real Consul: register both schemas, remove both registrations through the agent API, and run three real TooZ heartbeat passes. Native registration returns; the legacy alias remains absent. This test simulated registration loss. It did not restart the agent or wait for automatic critical-check deregistration.\n\nPlease retain the full alias registration data and reconcile missing aliases while the corresponding listener remains registered. Use bounded API calls and integrate with the existing listener lifecycle. Do not blindly re-register a healthy alias every cycle: register() currently resets its health status to \u0027passing\u0027. Ensure reconciliation cannot recreate an alias after unregister/stop, and that forked children do not incorrectly take ownership of a parent\u0027s registrations.\n\nRegression coverage: remove an alias from real Consul and verify it reappears and is discoverable with the legacy schema without restarting the listener. Also exercise agent restart/recovery and an unregister-versus-reconciliation race; after shutdown neither schema should be recreated.","commit_id":"badc646943b220279a3c4610da90126e34974703"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":true,"context_lines":[{"line_number":1272,"context_line":""},{"line_number":1273,"context_line":"        legacy_failed \u003d False"},{"line_number":1274,"context_line":"        if self._legacy_catalog is not None:"},{"line_number":1275,"context_line":"            service_name \u003d group_name[len(GROUP_PREFIX):].decode()"},{"line_number":1276,"context_line":"            try:"},{"line_number":1277,"context_line":"                entries \u003d self._legacy_catalog.get_services("},{"line_number":1278,"context_line":"                    service_name, server\u003dserver, fetch_all\u003dfetch_all)"}],"source_content_type":"text/x-python","patch_set":1,"id":"e05f4397_e2c19ab2","line":1275,"updated":"2026-09-10 03:17:33.000000000","message":"[P2] Derive legacy catalog names from the original exchange and topic\n\nThe normalized native service name is not always the name used by the old catalog. The legacy _service_name() in 973843 PS14 always prepends exchange to topic. The modern helper avoids adding an exchange prefix that is already present in the topic. Stripping GROUP_PREFIX here cannot recover the original inputs. Registration at lines 813-815 has the same mismatch.\n\nConcrete reproduction with no transport namespace: exchange\u003d\u0027review\u0027, topic\u003d\u0027review.prefixed\u0027. The old code searches \u0027review.review.prefixed\u0027, but the bridge registers and queries \u0027review.prefixed\u0027. A real Consul lookup using the old name finds no target. This is a valid topic case; whether it affects LYCorp immediately depends on the topics actually deployed.\n\nPlease preserve enough original namespace/exchange/topic information to derive both catalog names with their respective naming rules. Apply that mapping consistently to alias registration and legacy discovery; do not change modern naming or try to infer the original topic solely from an already-normalized string.\n\nRegression coverage: compare both naming helpers for ordinary and already-prefixed topics, with and without a transport namespace. Use real Consul to show that an old client\u0027s lookup finds the upgraded listener and that a new client can find a legacy-only listener, without cross-namespace matches. Please also check the actual deployed service topics against this mapping.","commit_id":"badc646943b220279a3c4610da90126e34974703"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"a460eccda8ee1467d979f338765da1aff24b4f18","unresolved":true,"context_lines":[{"line_number":1297,"context_line":"            seen.add(key)"},{"line_number":1298,"context_line":"            deduplicated.append(service)"},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":"        if not deduplicated and (tooz_failed or legacy_failed):"},{"line_number":1301,"context_line":"            return _FETCH_FAILED"},{"line_number":1302,"context_line":"        return deduplicated"}],"source_content_type":"text/x-python","patch_set":1,"id":"59bb725b_2da7c7dc","line":1300,"updated":"2026-09-10 03:17:33.000000000","message":"[P1] Do not treat an incomplete catalog merge as complete fanout discovery\n\nA non-empty result does not establish that discovery was complete. If the legacy lookup fails while native members exist, this condition returns the native-only list as success. The broadcaster can then admit a fanout that silently omits legacy-only recipients. The inverse failure direction has the same risk.\n\nReproduced on PS1 with real Consul containing one native/dual-registered endpoint and one legacy-only endpoint. A local fault proxy returns HTTP 503 only for the legacy service lookup. With fetch_all\u003dTrue, _fetch_members() returns one endpoint instead of _FETCH_FAILED, although the complete target set has two.\n\nPlease track completeness separately from the available endpoints. For fanout, do not admit an incomplete fresh snapshot as successful discovery. Propagate failure into the existing error/cache path without overwriting a previously complete snapshot with partial data. If using a previous complete snapshot is intended, make that policy explicit and bounded. Non-directed unicast may still use an available endpoint; do not impose fanout\u0027s completeness requirement indiscriminately.\n\nAlso fix _LegacyConsulCatalog.get_services(): its \u0027if not services and last_error\u0027 condition at lines 263-265 hides failed datacenter lookups when another datacenter returns members; failed datacenter enumeration can hide remote recipients too.\n\nTests should cover each catalog failing independently, partial multi-DC failure, no-cache and cached cases, and recovery. Assert the broadcaster does not acknowledge incomplete fanout as successful, while healthy unicast fallback and native-preferred deduplication still work.","commit_id":"badc646943b220279a3c4610da90126e34974703"}]}
