[PR #1] [MERGED] Derive max_poll_attempts from hard_timeout_s instead of reading wire field #3

New issue

Closed

opened 2026-10-26 22:28:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:28:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-remote-nodes/pull/1
Author: @Kosinkadink
Created: 5/13/2026
Status: ✅ Merged
Merged: 5/13/2026
Merged by: @Kosinkadink

Base: feat/rnp-phaseA-svg-autogrow-dynamiccombo ← Head: feat/derive-poll-attempts-from-timeout

📝 Commits (1)

11a84be Derive max_poll_attempts from hard_timeout_s instead of reading wire field

📊 Changes

2 files changed (+39 additions, -12 deletions)

View changed files

📝 protocol.py (+12 -1)
📝 proxy_node.py (+27 -11)

📄 Description

Pairs with comfy-rnp-server PR #1 which dropped descriptor.remote.execution.max_poll_attempts. The client now derives its poll-loop cap as ceil((hard_timeout_s + DEFAULT_POLL_GRACE_S) / poll_interval_s) inside _extract_execution_policy.

Why: coupling a contract bound to a cadence value silently breaks every time you tune the cadence. A descriptor saying 150 attempts shrinks 4× if poll_interval_s drops 2.0 → 0.5; expressing the bound as a duration (hard_timeout_s) keeps it invariant. Industry precedent: OAuth2 device flow §3.5, Kubernetes --timeout, gRPC deadlines all express long-poll bounds as durations rather than counts.

Changes:

protocol.py: drop DEFAULT_MAX_POLL_ATTEMPTS, add DEFAULT_SOFT_TIMEOUT_S / DEFAULT_HARD_TIMEOUT_S / DEFAULT_POLL_GRACE_S.
proxy_node.py: import math, derive max_poll_attempts inline in _extract_execution_policy from hard_timeout_s + grace / poll_interval_s. Falls back to soft_timeout_s when hard_timeout_s is absent, then to DEFAULT_HARD_TIMEOUT_S.

Base note: this PR targets feat/rnp-phaseA-svg-autogrow-dynamiccombo because the protocol change builds on the Phase A descriptor parser additions. When Phase A merges to main, this protocol cleanup goes with it.

No compat shim — RNP/1 is prototype-stage with zero backward-compat obligation (see comfy-rnp-server AGENTS.md). Server and client roll in lockstep.

Verified end-to-end with notes/run_url_fetch_executor.py, notes/run_recraft_custom_io_executor.py, and the paid notes/run_recraft_text_to_image_smoke.py (~$0.04) after restarting both processes with the new code on both sides.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-remote-nodes/pull/1 **Author:** [@Kosinkadink](https://github.com/Kosinkadink) **Created:** 5/13/2026 **Status:** ✅ Merged **Merged:** 5/13/2026 **Merged by:** [@Kosinkadink](https://github.com/Kosinkadink) **Base:** `feat/rnp-phaseA-svg-autogrow-dynamiccombo` ← **Head:** `feat/derive-poll-attempts-from-timeout` --- ### 📝 Commits (1) - [`11a84be`](https://github.com/Comfy-Org/comfy-remote-nodes/commit/11a84be06d285a5e72c6215b3567ca5176732cda) Derive max_poll_attempts from hard_timeout_s instead of reading wire field ### 📊 Changes **2 files changed** (+39 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `protocol.py` (+12 -1) 📝 `proxy_node.py` (+27 -11) </details> ### 📄 Description **Pairs with** comfy-rnp-server PR #1 which dropped `descriptor.remote.execution.max_poll_attempts`. The client now derives its poll-loop cap as `ceil((hard_timeout_s + DEFAULT_POLL_GRACE_S) / poll_interval_s)` inside `_extract_execution_policy`. **Why**: coupling a contract bound to a cadence value silently breaks every time you tune the cadence. A descriptor saying `150 attempts` shrinks 4× if `poll_interval_s` drops 2.0 → 0.5; expressing the bound as a duration (`hard_timeout_s`) keeps it invariant. Industry precedent: OAuth2 device flow §3.5, Kubernetes `--timeout`, gRPC deadlines all express long-poll bounds as durations rather than counts. **Changes**: - `protocol.py`: drop `DEFAULT_MAX_POLL_ATTEMPTS`, add `DEFAULT_SOFT_TIMEOUT_S` / `DEFAULT_HARD_TIMEOUT_S` / `DEFAULT_POLL_GRACE_S`. - `proxy_node.py`: import `math`, derive `max_poll_attempts` inline in `_extract_execution_policy` from `hard_timeout_s` + grace / `poll_interval_s`. Falls back to `soft_timeout_s` when `hard_timeout_s` is absent, then to `DEFAULT_HARD_TIMEOUT_S`. **Base note**: this PR targets `feat/rnp-phaseA-svg-autogrow-dynamiccombo` because the protocol change builds on the Phase A descriptor parser additions. When Phase A merges to main, this protocol cleanup goes with it. **No compat shim** — RNP/1 is prototype-stage with zero backward-compat obligation (see comfy-rnp-server AGENTS.md). Server and client roll in lockstep. **Verified** end-to-end with `notes/run_url_fetch_executor.py`, `notes/run_recraft_custom_io_executor.py`, and the paid `notes/run_recraft_text_to_image_smoke.py` (~$0.04) after restarting both processes with the new code on both sides. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:28:49 +00:00