Feature/api cleanup

[reiase]

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes GitHub issue: #xxx

In general, empty except blocks should either be removed (if exceptions should propagate), narrowed, or augmented with at least logging or a comment explaining why it is safe to ignore them. Here, we want to preserve existing behavior: exceptions from inspect.getfile(cls) should not break registration, but should be visible for debugging.

The best fix is to keep the try/except but add a debug-level log entry in the except block explaining that the source file could not be determined, including the exception details. This keeps the code running exactly as before (no new exceptions are raised; metadata remains optional) while satisfying the static analysis rule and improving debuggability. Concretely, in python/pulsing/actor/remote.py, lines 78–82 inside _register_actor_metadata, replace the bare pass in the except (TypeError, OSError): block with a call to the module-level logger (already defined at line 38). No new imports are needed, since logging is already imported and logger is initialized.

To fix an unused import, remove the import statement for the unused module. This eliminates an unnecessary dependency, cleans up the code, and resolves the static analysis warning.

In this file (test_actor_list_integration.py), the best fix is to delete the import time line (line 6) and leave the rest of the file unchanged. No additional code, imports, or definitions are required, because all timing behavior is already handled through asyncio.sleep, and time is never referenced. The only region to edit is the import block at the top of the file: remove the import time line and keep the other imports (asyncio, subprocess, init, remote) as-is.

To fix the unused import problem, remove the import of list_actors since it is not referenced anywhere in this test. In general, unused imports should be deleted unless they are required for module-level side effects, which is not the case here.

Specifically, in test_actor_list_same_process.py, delete line 12: from pulsing.admin import list_actors. No other code needs to change, because all functionality (creating actors, listing them via system.local_actor_names(), and printing them) does not depend on list_actors. The remaining imports (asyncio, sys, os, and from pulsing.actor import init, remote, get_system) are still required by the existing code.

In general, to fix an unused local variable where the right-hand side has important side effects, you remove just the variable binding and keep evaluating the expression, or rename the variable to an accepted “intentionally unused” pattern. Here, the side effects we care about are the creation/registration of remote actors via Counter.remote and Calculator.remote. The variables _counter1, _counter2, and _calc are never used after creation, so the simplest behavior-preserving fix is to await those calls without assigning the results.

Concretely, in test_actor_system.py, lines 29–31 should be updated to drop the variable names and just await the remote calls. This keeps the actor-creation side effects (and the naming of the actors via name="...") exactly as before, while eliminating the unused local variables that CodeQL reports. No additional imports, methods, or definitions are required.

In general, to fix an unused local variable warning when the assignment’s right-hand side has important side effects, you should remove the unused variable binding while keeping the expression that causes the side effect. Alternatively, if the variable is intentionally unused for clarity or documentation purposes, you can rename it to match an accepted “intentionally unused” pattern so static analysis tools ignore it.

Here, the best way without changing functionality is to drop the unused target _counter2 and keep the awaited call to Counter.remote. That means replacing _counter2 = await Counter.remote(...) with simply await Counter.remote(...). This preserves the creation and registration of the counter-2 actor while removing the unused local variable altogether. We should apply the same treatment consistently to _counter1 and _calc, since they are equally unused in this snippet, but per the prompt we must at least address _counter2. No new imports, methods, or definitions are required.

Concretely in test_actor_system.py, within main() at lines 29–31, we will change:

• Line 29: _counter1 = await Counter.remote(name="counter-1") → await Counter.remote(name="counter-1")
• Line 30: _counter2 = await Counter.remote(name="counter-2") → await Counter.remote(name="counter-2")
• Line 31: _calc = await Calculator.remote(name="calculator") → await Calculator.remote(name="calculator")

This keeps all actors being created as before while eliminating unused locals.

In general, to fix an unused local variable where the right-hand side has important side effects, remove or rename the left-hand side binding while preserving the expression evaluation. If the variable is truly needed later, instead use it somewhere meaningful; if it is intentionally unused, rename it to an accepted “unused” pattern.

Here, the right-hand side await Calculator.remote(name="calculator") likely has side effects (creating/registering the actor), so we should not remove the call. The variable _calc itself is never used, so the simplest and safest fix is to remove the assignment target and just await the call as a standalone statement. Concretely, in test_actor_system.py line 31, change:

    _calc = await Calculator.remote(name="calculator")

to:

    await Calculator.remote(name="calculator")

This keeps existing functionality (actors get created) while eliminating the unused local variable and satisfying CodeQL. No new imports, methods, or definitions are needed.

In general, empty except blocks should either be removed, narrowed to specific expected exceptions, or should handle/log the exception in a meaningful way. In test teardown/cleanup code, a common pattern is to log the exception while allowing the test suite to proceed.

The best fix here without changing intended functionality is to keep catching Exception (so teardown failures don’t break tests) but to log the exception with a clear message. Pytest’s logging integration works with the standard library logging module, which is widely used and already available. We will:

• Add an import logging alongside the existing imports at the top of tests/python/conftest.py.
• Replace the bare except Exception: pass with an except Exception as exc: block that uses logging.getLogger(__name__).warning(..., exc_info=True) to record the failure and stack trace. This preserves the behavior of “don’t raise” while no longer silently discarding information.

Concretely:

• Modify lines around the imports to add import logging.
• Modify lines 29–30 so that instead of except Exception:\n pass, we log the exception and do not re-raise.

To fix the problem, remove the unused import statement for asyncio from the top of tests/python/test_actor_list.py. This resolves the unused import without changing any behavior, because the module and its contents were never referenced.

Concretely, in tests/python/test_actor_list.py, delete line 3 (import asyncio) and leave the remaining imports (pytest, pulsing.actor, pulsing.cli.actor_list, io, sys) unchanged. No new methods, imports, or definitions are required.

To fix an unused import, remove the import statement for the module that is not referenced anywhere in the file. This reduces unnecessary dependencies and makes the code clearer without altering behavior.

In this case, the only change needed is to delete the import asyncio line near the top of tests/python/test_rest_api.py. No other code adjustments are required because nothing in the file references asyncio. Specifically, remove line 3 (import asyncio) and leave the remaining imports (pytest, unittest.mock, json, subprocess, and the pulsing.cli.actor_list items) unchanged. This will not affect test functionality since those imports are the ones actually used by the tests.

To fix an unused import, remove the import statement for the unused module. This eliminates an unnecessary dependency and has no effect on runtime behavior as long as the module is truly unused.

In this case, the best fix is to delete the import subprocess line near the top of tests/python/test_rest_api.py. All mocking of subprocess behavior is already done via unittest.mock.patch with the fully qualified target string "subprocess.run", which does not require the subprocess name to be imported into this module. No other code in the shown regions refers to subprocess, and CodeQL confirms that there are no references elsewhere in the file.

Concretely, in tests/python/test_rest_api.py, remove line 7 containing import subprocess and leave all other imports and code unchanged. No additional methods, imports, or definitions are needed.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 67.71488% with 154 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/pulsing-actor/src/cluster/gossip.rs	22.58%	48 Missing ⚠️
python/pulsing/cli/actor_list.py	75.65%	37 Missing ⚠️
crates/pulsing-actor/src/system.rs	13.33%	26 Missing ⚠️
crates/pulsing-actor/src/transport/http2/server.rs	14.81%	23 Missing ⚠️
crates/pulsing-actor/src/tracing/mod.rs	0.00%	7 Missing ⚠️
python/pulsing/actor/remote.py	77.77%	6 Missing ⚠️
python/pulsing/cli/__main__.py	0.00%	4 Missing ⚠️
python/pulsing/admin.py	0.00%	2 Missing ⚠️
crates/pulsing-actor/src/system_actor/mod.rs	50.00%	1 Missing ⚠️

Files with missing lines	Coverage Δ
crates/pulsing-actor/src/cluster/member.rs	100.00% <100.00%> (ø)
crates/pulsing-actor/src/system_actor/messages.rs	92.30% <100.00%> (+0.15%)	⬆️
python/pulsing/actor/__init__.py	80.39% <ø> (+3.92%)	⬆️
crates/pulsing-actor/src/system_actor/mod.rs	71.07% <50.00%> (-0.21%)	⬇️
python/pulsing/admin.py	0.00% <0.00%> (ø)
python/pulsing/cli/__main__.py	0.00% <0.00%> (ø)
python/pulsing/actor/remote.py	59.91% <77.77%> (+5.03%)	⬆️
crates/pulsing-actor/src/tracing/mod.rs	60.46% <0.00%> (-1.82%)	⬇️
crates/pulsing-actor/src/transport/http2/server.rs	64.79% <14.81%> (-3.54%)	⬇️
crates/pulsing-actor/src/system.rs	69.54% <13.33%> (-2.38%)	⬇️
... and 2 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.