refactor(connection): remove RedisConnectionFactory.connect and the url= keyword

[vishal-bala]

Motivation

RedisConnectionFactory.connect() was a thin dispatcher over get_redis_connection and get_async_redis_connection, deprecated since v0.4.0, with no callers left anywhere in the library. The url keyword on the two async factories was deprecated in v0.11.0 in favour of redis_url.

This is the third and last of three stacked pull requests removing the deprecated client and connection surface. With it, a Redis client reaches an index through exactly one route: the constructor, either as redis_client or as redis_url.

Changes

url is rejected, not merely dropped

Left unguarded, url= would reach is_cluster_url, which takes url positionally, and raise got multiple values for argument 'url' — an error naming neither RedisVL nor the rename. If REDIS_URL happened to be unset, a ValueError about that variable would arrive first instead. All five factory entry points now name the replacement.

That count matters: the two cluster factories, get_redis_cluster_connection and get_async_redis_cluster_connection, had never carried the deprecated decorator but accepted url by the same keyword-forwarding accident, so guarding only the decorated ones would have left the obscure failure reachable through exactly the entry points a cluster user is most likely to call by hand.

The removed-keyword table splits in two along entry points rather than gaining a fourth shared entry. url belongs to the connection factories and connection_args to the index constructors, and one shared table would have told a factory caller that connection_args had been withdrawn when that door never accepted it.

get_async_redis_connection now tells the truth about itself

It survives, but three things about it were wrong. It warned before validating, so a call about to be rejected still emitted advisory noise — and under -W error the warning, not the helpful TypeError, would have been the surfaced error. Its warning had no stacklevel, so it pointed at this module rather than at the caller. And its text said the function "will become async", which describes the wrong difference: what actually distinguishes it from _get_aredis_connection is that it is the one factory that never sends CLIENT SETINFO, so its client reports no library name and defers its first connection to the first command.

The docstring now says that, and says plainly that the function is not being removed and has no replacement to migrate to, rather than pointing users at a private sibling.

A latent reconnection path, opened by owns_client

disconnect() nulls an owned client and the lazy accessor then re-creates one. For a client handed over with owns_client=True there is no redis_url to rebuild from, so creation fell through to REDIS_URL and could silently connect to a different server than the caller supplied. Both accessors now refuse, naming the reason. The no-argument constructor still resolves from the environment, which is a documented path.

This was unreachable before the first branch in this stack, because redis_client= implied not-owned and disconnect() returned early without nulling anything.

Documentation that contradicted the code

get_redis_connection documented a parameter named url that its signature does not have and that this change makes fatal, and all three factories told readers in Raises: to pass it. None documented the new TypeError. The two tables also lost their leading underscore when the table split, publishing module-level names that feed a private helper.

Notes

The url keyword had a sharper consequence than an unhelpful error, and it is worth recording because it is now unreachable. _get_aredis_connection popped url and gave it precedence over redis_url, and AsyncSearchIndex forwards connection_kwargs verbatim into that factory. So AsyncSearchIndex(redis_url="redis://intended", connection_kwargs={"url": "redis://elsewhere"}) connected to the second address — connection-target confusion through a keyword the caller believed was deprecated and inert. The sync path already failed loudly, so this was async-only.

get_async_redis_connection's signature-change notice was asserted nowhere after the url-deprecation tests were deleted; that file's only load-bearing assertion was the warning, despite its name suggesting otherwise. It is pinned again, so the warning cannot be deleted or reworded with the suite still green.

Of the five tests removed here, two only asserted that the deleted dispatcher returned a sync or an async client, which every fixture in the suite already does. The three URL-resolution error paths it covered survive on get_redis_connection and are covered nowhere else, so they moved rather than went. One of them restored REDIS_URL outside a finally, so a failed assertion would have left the variable unset and cascaded into every later test that resolves its URL from the environment.

Whether get_async_redis_connection should become a coroutine, which is what its warning has been promising, is now a small and well-scoped decision: it has zero callers inside the library, every remaining reference is a test, and the async implementation already exists as _get_aredis_connection.

The RESP2 pin these factories apply is unchanged and remains correct: RESP3 changes FT.SEARCH and FT.AGGREGATE from an array to a map, which is the shape RedisVL parses. Worth knowing what it costs, though, since setdefault leaves protocol=3 available to callers who want it: client-side caching and Smart Client Handoffs both require RESP3, so a user on Redis Cloud takes full disconnections during planned maintenance that a RESP3 client would ride through.

Release Notes

RedisConnectionFactory.connect() is removed. Use get_redis_connection() or get_async_redis_connection() directly.

The url keyword is removed from the connection factories in favour of redis_url. Passing it now raises TypeError naming the replacement, on all five factory functions, rather than failing with an unrelated error from redis-py. Note that url previously took precedence over redis_url on the async factory, so an AsyncSearchIndex built with both redis_url and a url inside connection_kwargs would connect to the latter; that combination now raises.