Repository navigation
pubsub: breaking changes to HTTP and client options - #739
Open
owenpearson wants to merge 8 commits into
Open
owenpearson wants to merge 8 commits into
owenpearson wants to merge 8 commits into
Conversation
RTC7 has a client use the timeouts configured in its ClientOptions, falling back to the library defaults, and the UTS test for it reads those defaults back from client.options. The realtime timeouts were already resolved on Options, but http_open_timeout, http_request_timeout, http_max_retry_count and http_max_retry_duration were left as None there and defaulted later, by Http from its CONNECTION_RETRY_DEFAULTS and by the host list from Defaults.http_max_retry_count, so client.options could not say what a client was going to use. The four defaults now live on Defaults and are resolved by Options, the same way as the realtime ones, and Http reads them from the options. CONNECTION_RETRY_DEFAULTS is gone. The values are unchanged and stay in seconds, so the TO3l3 and TO3l4 unit mismatch remains recorded as a deviation in test/uts/deviations.md. This is a breaking change for code that read these options and took None to mean unset: they now read 4, 10, 3 and 15 by default. UPDATING.md says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
time(), stats(), the REST channel's publish variants, get_message() on the REST and realtime channels and push.admin.publish() each accepted a timeout keyword argument and passed it down to Http.make_request, which replaced it with the client's (http_open_timeout, http_request_timeout) before building the request. So the argument has never had any effect. stats() also took a paginated argument that nothing read. Both arguments are removed, from the clients, the prototypes and the Http get, post, put, patch and delete helpers and make_request, which no caller needs them for. RTC7 has the timeouts come from the ClientOptions, which is where they were always taken from. This is a breaking change: passing timeout or paginated now raises TypeError. Callers should drop the argument and set http_request_timeout or http_open_timeout on the client instead, as UPDATING.md describes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Http.make_request passed httpx the tuple (http_open_timeout, http_request_timeout), which httpx expands to a connect and a read timeout and leaves write and pool as None. Writing a request body to a wedged connection, or waiting for a free pooled connection, therefore never timed out, whatever the client's options said. This is one of the three parts of issue #709. The timeout is now an explicit httpx.Timeout: connect takes http_open_timeout (TO3l3), and read, write and pool take http_request_timeout (TO3l4). The other parts of #709, that the read timeout applies per socket read and that the RSC15 retry budget is not consulted while an attempt is in flight, are left as they are, as is the unit of both options, which stays seconds. This is a breaking change in behaviour: a request that used to hang in either phase now raises httpx.TimeoutException after http_request_timeout and is retried against a fallback host. Callers who send large bodies over slow links may need to raise http_request_timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TO3l5 defines httpMaxRetryCount as the maximum number of fallback hosts to use when a request to the primary host fails. Options built the host list by truncating the primary plus the fallbacks to http_max_retry_count, so it counted the primary too: the default of 3 tried the primary and two fallbacks, one host fewer than ably-js, and http_max_retry_count=0 left the list empty, so every request failed with an AttributeError wrapped as a 50000. The host list is now the primary followed by at most http_max_retry_count of the shuffled fallback hosts, so the default tries four hosts and 0 tries the primary alone. RSC15a's row in test/uts/deviations.md called the old truncation intentional; it now records the four hosts as compliant. This is a breaking change: a client tries one more host per request for the same setting. Callers who want the 3.x number of attempts should pass one less, as UPDATING.md shows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TO3b leaves the implementation of logLevel to the platform. ably-python
accepted a log_level option, stored it on Options and gave it a property
and a setter, but nothing read it, so setting it changed nothing. The
library has always logged through the standard logging module, under the
ably.pubsub logger, which is where Python applications expect to set a
level or attach a handler.
The option is removed, and the factory docstrings now say how to configure
the ably.pubsub logger instead. No log handler option is added. The RSC2,
RSC3, RSC4, TO3b, TO3c and TO3c2 tests stay gated, and their row in
test/uts/deviations.md now records that neither option exists.
This is a breaking change: passing log_level now raises TypeError. Callers
should drop it and configure logging.getLogger('ably.pubsub') instead, as
UPDATING.md shows.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
environment, rest_host and realtime_host were deprecated in favour of endpoint in 3.0.0, and TO3k1, TO3k2 and TO3k3 let a library with no backwards-compatibility requirement omit them. Options still accepted all three, mapped them onto endpoint with a warning, and carried the REC1b1 and REC1c1 conflict checks between them. The client options also took the token under two names, token (TO3j2), which the factories document, and auth_token, which the http client used internally and passed through from callers unchecked. And Options had a connection_state_ttl property and setter that copied Defaults.connection_state_ttl and that nothing read; the realtime client's docstring documented a connection_state_ttl keyword the constructor has never accepted. All of these are removed, along with the docstrings that mentioned them. endpoint is the one way to name a host, and the token option is token, read back as options.token. The UTS tests for REC1b1, REC1c, REC1d, REC2c5 and REC2c6 that exist only to exercise the removed host options are skipped as not applicable, and the four that used one only to name a host use endpoint instead; test/uts/deviations.md records both and updates its counts. This is a breaking change: passing environment, rest_host, realtime_host or auth_token now raises TypeError, and options.auth_token and options.connection_state_ttl are gone. Callers should pass endpoint for the first three and token for the fourth, as UPDATING.md shows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TO3l8 makes maxMessageSize a client option, default 65536, and RSL1i has a publish whose messages exceed it between them rejected with 40009 before it is sent. ably-python had no such option, and REST publishes were never checked. The realtime publish read the limit with getattr(options, 'max_message_size', 65536), which always fell back to the default, and measured the whole encoded message, msgpack or JSON with ASCII escaping, rather than the TM6 size, so it counted the id, the encoding and every other field the specification leaves out. max_message_size is now a client option, defaulting to Defaults.max_message_size. message_size measures a message per TM6: the lengths of its name, clientId and string data, the bytes of binary or encrypted data, and the length of the JSON of object data and extras. The REST and realtime publishes both check their messages with it, after encryption and before encoding. CD2c, the connectionDetails override for realtime, is not implemented, since ably-python does not parse that field. The RSL1i UTS test is un-gated. Its at-limit case is five bytes over once TM6 counts the name, so the derived test corrects the fixture, which test/uts/deviations.md records as a spec error along with the updated counts. This is a breaking change: a REST publish over 64 KiB now raises an AblyException with code 40009 instead of being sent, and some realtime publishes that were refused are now sent. Callers whose account allows larger messages can raise max_message_size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TO3i's recover option names a recovery key, from which RTN16 has a client inherit the state of an earlier connection. ably-python does not implement connection recovery: it accepted recover, documented it on the realtime factory, and stored it with a property and a setter, but never sent it, so a client given a recovery key silently started a fresh connection. The option, its property and its docstring are removed. The RTN16 and RTC1c tests stay gated, and the RTN16f1 malformed-key test, which passed only because the key was ignored, is gated with them; test/uts/deviations.md records the change and updates its counts. This is a breaking change: passing recover now raises TypeError. Callers should drop it. Connection resumption (RTN15) is unaffected, and connection recovery may return, with the option, in a minor release, as UPDATING.md notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 32 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Oct 26, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Eight breaking changes to the HTTP layer and the client options for 4.0.0, one commit each. Every commit has an entry in
UPDATING.md, updatestest/uts/deviations.mdwhere it touches a recorded deviation, and passesruffandtest/utson its own.Moving durations between seconds and milliseconds is deliberately left out, pending discussion.
http_open_timeout,http_request_timeoutandhttp_max_retry_durationstay in seconds, the realtime timeouts stay in milliseconds, and token ttl is untouched. The TO3l3/TO3l4 unit mismatch therefore stays recorded as a deviation.Commits
http_open_timeout,http_request_timeout,http_max_retry_countandhttp_max_retry_durationare resolved onOptionsfromDefaults, the way the realtime timeouts already are, so they read back 4, 10, 3 and 15 instead ofNone.Http.CONNECTION_RETRY_DEFAULTSis removed. Migration: code that tookNoneto mean "unset" will no longer see it.timeout=argument. It was accepted bytime(),stats(), the REST publish variants,get_message()on REST and realtime channels andpush.admin.publish(), andmake_requestalways overwrote it. It is removed from those methods, the prototypes and theHttphelpers, along withstats()'s unusedpaginated=. Migration: drop the argument (it now raisesTypeError) and sethttp_request_timeouton the client.httpx.Timeout(http_request_timeout, connect=http_open_timeout), so write and pool are bounded byhttp_request_timeoutinstead of beingNone. This is one part of http_request_timeout does not bound a REST request #709; the per-read timeout and the in-flight retry budget are not addressed. Migration: a request stuck writing or waiting for a pooled connection now times out and falls back. Raisehttp_request_timeoutif large bodies on slow links need longer.http_max_retry_countcounts fallback hosts only (TO3l5). The host list is now the primary plus up tohttp_max_retry_countfallbacks. The default tries four hosts, as ably-js does, and0tries the primary only instead of failing every request. The RSC15a row that called the old truncation "Intentional" is corrected. Migration: pass one less to keep the 3.x number of attempts.log_leveloption. It was stored and never read. The factory docstrings now point at theably.pubsublogger. No log handler option is added. Migration: configurelogging.getLogger('ably.pubsub').environment,rest_hostandrealtime_host(TO3k1-3) are removed; useendpoint. The duplicateauth_tokenis removed; usetoken(TO3j2), which now reads back asoptions.token. The deadOptions.connection_state_ttlproperty and the realtime docstring for aconnection_state_ttlkeyword that never existed are also removed. The UTS tests that exist only to exercise the removed host options (12 Test IDs under REC1b1, REC1c, REC1d, REC2c5 and REC2c6) are skipped as not applicable. Four others now useendpointto name their host.max_message_size(TO3l8, RSL1i, TM6).max_message_sizeis now a client option, defaulting to 65536.message_sizemeasures a message per TM6. REST publishes are now checked and raise 40009, and realtime publishes use the same measurement, which no longer counts the id, the encoding or JSON escaping. The RSL1i UTS test is un-gated. Its at-limit fixture is five bytes over once TM6 counts the name, so the test corrects the fixture and the fault is recorded as a spec error. Migration: REST publishes over the limit now fail locally; raisemax_message_sizeif the account allows.recoveroption. Connection recovery (RTN16) is not implemented, and the option was accepted, documented and ignored. Passing it now raisesTypeError. The RTN16f1 malformed-key test, which passed only because the key was ignored, is gated with the other recovery tests.UPDATING.mdnotes that recovery may return in a minor release. Migration: drop the option.Scoped down or skipped
connectionDetails.maxMessageSizewas not used on realtime before this PR, andConnectionDetailsdoes not parse it, so it is still not used. The TO3l8 row indeviations.mdnow records CD2c alone as open.PresenceMessage.size) and presence size checks are not added. The TP5 test stays gated.options. They still record the seconds-versus-milliseconds unit as a deviation.Tests
test/uts, measured at the base and at the head of this branch:7032b4cRUN_DEVIATIONSRUN_DEVIATIONS=1Each commit was also run through
test/utsboth ways, withruff, and each matches the counts its owndeviations.mdstates. The first five leave the counts at the base figures.The twelve newly skipped tests are the omitted-host-option ones. Under
RUN_DEVIATIONS, RSL1i passes and RTN16f1 fails, and every other failure is unchanged. Thedeviations.mdheader and its measured-counts section are updated to these figures.New unit tests are in
test/unit: the httpx timeout phases, the TO3l5 host counts (including0), TM6 sizes and 40009, and the removed options raisingTypeError.The legacy
test/ablysuite was not run.test/ably/http/httphttp_test.pyandtest/ably/http/httpinit_test.pywere edited by hand to follow these changes and want a CI run.🤖 Generated with Claude Code