EG-4662 (12): Share `_InstanceRegistry` using a singleton by ppicom · Pull Request #423 · Unleash/unleash-python-sdk

ppicom · 2026-08-20T06:14:02Z

Part of what the SDK does is registering how many copies of a client you've got running. There's different modes in which this can run:

We don't let you run more than one copy
We warn you
We do nothing (and let you do whatever you want)

The changes in this PR move the logic of registering an SDK as "in use" to a new InstanceRegistry class. SDKs will use it as a singleton so that there is only one count of connected SDKs per runtime, regardless of what combination of SDKs the user decides to use.

NOTE: The original client did not de-register clients, which is something that the Copilot review has brought up. I'd leave it as a possible improvement after an initial review because I don't see it as a big issue. But I'm open to feedback on that as well.

feat: move instance registration out of utils and make it a singleton
refactor: replace InstanceCounter in UnleashClient by InstanceRegistry singleton
feat: wire async client with InstanceRegistry
docs: tentative changelog update

…y singleton

Copilot

Copilot review overview

🟡 Changes recommended

Resolve the critical async registration and positional API compatibility issues before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 · 1 · 1

Open (4)

New parameter breaks existing positional argument compatibility · New
Unfinished async clients incorrectly affect shared instance limits · New
Registry retains destroyed clients without unregistering · New
Describe current async client behavior, not unimplemented registration · New

What changed in this PR

Moves duplicate-instance tracking into a shared singleton registry for synchronous and asynchronous clients.

Changes:

Replaces InstanceCounter with _InstanceRegistry.
Adds async-client registration and duplicate-instance modes.
Updates tests and changelog documentation.

File	Summary
`UnleashClient/utils.py`	Removes the old instance counter.
`UnleashClient/clients/unleash_client.py`	Uses the shared registry for synchronous clients.
`UnleashClient/clients/async_unleash_client.py`	Adds async registration; critical issues remain around unfinished lifecycle methods and positional API compatibility (1 and 2 votes).
`UnleashClient/_instance_registry.py`	Implements the singleton registry; entries are never removed after client destruction (moderate, 2 votes).
`tests/unit_tests/test_instance_registry.py`	Tests registry behavior and singleton identity.
`tests/unit_tests/clients/test_async_unleash_client.py`	Tests async duplicate-instance behavior.
`CHANGELOG.md`	Documents the changes; the async behavior description is inaccurate (nit, 2 votes).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

ppicom · 2026-09-25T07:43:53Z

+        _get_instance().register(
+            identifier=self._config.instance_identifier, mode=multiple_instance_mode
+        )


Valid point, registration should happen at initialization.

alexanto · 2026-09-25T08:41:30Z

        self._closed = threading.Event()

-        self._do_instance_check(multiple_instance_mode)
+        _get_instance().register(


Suggested change

_get_instance().register(

INSTANCES.register(

The idea is that using _get_instance() left the door open to lazy initialization, but I did not implement it. 🤔 I think I prefer to leave the function in place for the moment. Sounds right?

alexanto · 2026-09-25T08:46:29Z

 * (Minor): The Sphinx documentation site is no longer built or published. It was served from a GitHub Pages custom domain that stopped resolving, so nothing published there had been reachable for some time. Reference documentation for this SDK is at https://docs.getunleash.io/reference/sdks/python, and `README.md` covers installation, usage, configuration options, custom strategies, custom caches, event callbacks and the WSGI and Celery notes. Contributor setup and the release checklist are in `DEVELOPMENT.md`. The `Documentation` URL in the package metadata now points at the docs site rather than a page that returned 404.
 * (Minor): `AsyncUnleashClient` ignores `custom_options`. The constructor still accepts it and it still reaches the shared configuration, but the async request path never passes it to the HTTP library, so the async client's surface does not depend on aiohttp's own keyword arguments. `UnleashClient` is unaffected and keeps passing `custom_options` to `requests`.
-* (Minor): The in-progress asynchronous client now builds an `AsyncMetricsReporter` over its `AsyncTransport`, and exposes `impact_metrics` like the synchronous client does. Nothing starts it yet — `initialize_client()` still raises `NotImplementedError` — so no metrics are sent, and nothing changes for code using `UnleashClient`. The request body, the impact-metrics collection and the restore-after-a-failed-send are shared with the synchronous reporter; only the request itself and the recurring schedule are separate.
+* (Minor): Duplicate-instance detection now happens through one internal `_InstanceRegistry`, in the private `UnleashClient._instance_registry` module, instead of an `InstanceCounter` and a private method on the client. The identifier a client is keyed by, the message, the exception raised under `InstanceAllowType.BLOCK`, the error logged under `InstanceAllowType.WARN` and the counting are exactly what they were, and a client rejected under `BLOCK` is still not counted. `INSTANCES` keeps its name, both its import paths and all of its methods, and remains the supported way to reach the registry. The module is not part of the public API and may change or disappear without notice. `multiple_instance_mode` is unchanged. `UnleashClient.utils.InstanceCounter` is gone, so this affects code importing that class directly. Nothing changes for code using `UnleashClient`.


should we be referencing internal modules in the changelog? 🤔

Uhmm right now the change log is generated, but it's not final. I am using it so we can later decide what goes into the final changelog + the docs.

But I'm open to better ways to do this 👍

Co-authored-by: Alexandra <szanto.szandra@gmail.com>

for more information, see https://pre-commit.ci

…-registry' into eg-4662-12-instance-registry

sighphyre

LGTM

thomasheartman added this to Issues and PRs Aug 20, 2026

github-project-automation Bot moved this to New in Issues and PRs Aug 20, 2026

ppicom changed the title ~~eg 4662 12 instance registry~~ EG-4662 (12): Share InstanceRegistry using a singleton Aug 20, 2026

ppicom force-pushed the eg-4662-12-instance-registry branch from 0e1732e to 4d52383 Compare August 20, 2026 06:56

ppicom force-pushed the eg-4662-12-instance-registry branch 2 times, most recently from 5aff4c5 to 7624b9d Compare August 20, 2026 07:29

ppicom force-pushed the eg-4662-12-instance-registry branch 2 times, most recently from a7a3450 to fa31283 Compare August 20, 2026 07:38

ppicom force-pushed the eg-4662-12-instance-registry branch from fa31283 to 05010c3 Compare August 20, 2026 08:26

ppicom force-pushed the eg-4662-12-instance-registry branch from 5b09f3b to aad3ead Compare August 20, 2026 09:21

gastonfournier moved this from New to In Progress in Issues and PRs Aug 21, 2026

ppicom force-pushed the eg-4662-12-instance-registry branch from aad3ead to a3ac936 Compare August 21, 2026 08:59

ppicom force-pushed the eg-4662-12-instance-registry branch from a3ac936 to afb7f7b Compare August 21, 2026 09:44

ppicom force-pushed the eg-4662-12-instance-registry branch from afb7f7b to bb76cf0 Compare September 21, 2026 13:33

ppicom force-pushed the eg-4662-12-instance-registry branch from bb76cf0 to d40073d Compare September 21, 2026 14:28

ppicom force-pushed the eg-4662-12-instance-registry branch 2 times, most recently from 1317629 to 51cc048 Compare September 23, 2026 06:45

ppicom force-pushed the eg-4662-12-instance-registry branch from b86cb9e to 779e820 Compare September 24, 2026 09:52

Base automatically changed from eg-4662-11-async-metrics-reporter to main September 24, 2026 12:58

ppicom added 3 commits September 24, 2026 14:58

feat: move instance registration out of utils and make it a singleton

e7d09b9

refactor: replace InstanceCounter in UnleashClient by InstanceRegistr…

31a1c82

…y singleton

feat: wire async client with InstanceRegistry

063acbf

ppicom force-pushed the eg-4662-12-instance-registry branch from 779e820 to 063acbf Compare September 24, 2026 12:58

ppicom added 3 commits September 25, 2026 08:54

docs: make comments more honest

afc04bc

docs: reword changelog

7bfbabe

refactor: make _InstanceRegistry private

40c810b

ppicom changed the title ~~EG-4662 (12): Share InstanceRegistry using a singleton~~ EG-4662 (12): Share _InstanceRegistry using a singleton Sep 25, 2026

ppicom requested review from alexanto and a lite review from Copilot September 25, 2026 07:30

Copilot started reviewing on behalf of ppicom September 25, 2026 07:30 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

adrian2x reviewed Sep 25, 2026

View reviewed changes

Comment thread UnleashClient/_instance_registry.py Outdated

Comment thread UnleashClient/_instance_registry.py Outdated

alexanto reviewed Sep 25, 2026

View reviewed changes

Comment thread UnleashClient/_instance_registry.py Outdated

alexanto reviewed Sep 25, 2026

View reviewed changes

Comment thread UnleashClient/_instance_registry.py Outdated

ppicom added 2 commits September 25, 2026 10:26

refactor: rename to _get_instance_registry

1185d2b

refactor: use the .get default param instead of or

274a302

alexanto reviewed Sep 25, 2026

View reviewed changes

Comment thread UnleashClient/_instance_registry.py Outdated

alexanto reviewed Sep 25, 2026

View reviewed changes

ppicom and others added 4 commits September 25, 2026 10:48

fix: return MultipleInstancesNotAllowedError instead of Exception

b760f87

makes it same style as count and shorter

15022a0

Co-authored-by: Alexandra <szanto.szandra@gmail.com>

[pre-commit.ci] auto fixes from pre-commit.com hooks

47236e2

for more information, see https://pre-commit.ci

Merge remote-tracking branch 'refs/remotes/origin/eg-4662-12-instance…

4be3796

…-registry' into eg-4662-12-instance-registry

sighphyre approved these changes Sep 25, 2026

View reviewed changes

ppicom merged commit cb28e83 into main Sep 25, 2026
8 checks passed

ppicom deleted the eg-4662-12-instance-registry branch September 25, 2026 13:24

Navigation Menu

EG-4662 (12): Share `_InstanceRegistry` using a singleton - #423

EG-4662 (12): Share `_InstanceRegistry` using a singleton#423
ppicom merged 12 commits into
mainfrom
eg-4662-12-instance-registry

ppicom commented Aug 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

ppicom Sep 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexanto Sep 25, 2026

Uh oh!

ppicom Sep 25, 2026

Uh oh!

alexanto Sep 25, 2026

Uh oh!

ppicom Sep 25, 2026

Uh oh!

sighphyre left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

ppicom commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

ppicom Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexanto Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

ppicom Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

alexanto Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

ppicom Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

sighphyre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ppicom commented Aug 20, 2026 •

edited

Loading