[s-celles-bot]

Issues Fixed

• Fixes [BUG] connect_to_redis() ignores REDIS_HOST: control API fails when Redis is not reachable as "redis" #3365

Description

connect_to_redis() always connected to the host name redis, while the channel layer in django_project/settings.py already reads REDIS_HOST. The host now comes from a small redis_host() helper that reads REDIS_HOST and defaults to redis, so nothing changes on the compose network.

The helper exists so the behaviour can be unit tested: conftest.py replaces connect_to_redis() with a fake for the whole session.

Checked with the screenly/anthias-server:0115908-x86 image, this utils.py mounted over the original, Redis and the server on the host network, REDIS_HOST=127.0.0.1: GET /api/v2/assets/control/next answers "Asset switched" (HTTP 200) instead of HTTP 500.

Checklist

• I have performed a self-review of my own code.
• New and existing unit tests pass locally and on CI with my changes. (Locally they pass; CI not run yet. Locally: tests/test_utils.py and the non-integration suite. test_assets_update_parses_12_hour_start_end_dates fails on master too unless TZ=UTC, unrelated.)
• I have done an end-to-end test for Raspberry Pi devices.
• I have tested my changes for x86 devices.
• I added a documentation for the changes I have made (when necessary).

Assisted-by: AI

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[vpetersson-bot]

Thanks for this — the diagnosis in #3365 is correct, and the fix is the right shape. I reviewed it and ran it on our hardware rig (x86, Pi5, Pi4, Pi3 64-bit, Pi3-32 and Pi2 armv7l, and a Radxa ROCK Pi 4B).

Confirmed working: with REDIS_HOST unset connect_to_redis() still resolves redis and pings fine, with it set to the Redis container IP it connects there, and with it set to a bogus host it raises ConnectionError — which proves the variable is genuinely used rather than silently ignored. port / db / decode_responses are unchanged. REDIS_HOST is set in none of the shipped compose / balena / ansible configs, so this is a no-op for existing deployments. Unit suite, ruff and mypy all clean.

Keeping the port hardcoded while making only the host configurable is the right call, by the way — it mirrors django_project/settings.py exactly, so the two stay symmetric.

Two things to fix:

1. The tests don't actually cover connect_to_redis(). Both new tests assert on redis_host() only, so nothing pins the thing the PR is actually about — if a future refactor dropped the redis_host() call from connect_to_redis() and went back to a literal 'redis', both tests would still pass. I verified the link by hand, but it's worth pinning. The conftest.py fake doesn't get in the way if you patch the constructor instead:

def test_connect_to_redis_uses_redis_host(monkeypatch: Any) -> None:
    monkeypatch.setenv('REDIS_HOST', '127.0.0.1')
    with patch.object(utils.redis, 'Redis') as mock_redis:
        utils.connect_to_redis()
    assert mock_redis.call_args.kwargs['host'] == '127.0.0.1'

2. This conflicts with your #3368. Both PRs append to the end of tests/test_utils.py, so whichever lands second needs a rebase. The resolution is just "keep both blocks" — I merged the two locally and the combined suite is green (2244 passing), so there's nothing subtle hiding in it.

Neither is a blocker on the fix itself.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@1244dca). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #3367   +/-   ##
=========================================
  Coverage          ?   90.75%           
=========================================
  Files             ?       85           
  Lines             ?    10475           
  Branches          ?     1163           
=========================================
  Hits              ?     9507           
  Misses            ?      708           
  Partials          ?      260           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[vpetersson-bot]

One more thing, and apologies that this isn't written down anywhere public yet — it's a house rule rather than something you could have inferred from CONTRIBUTING.md:

All commits need to be signed with an SSH key. This commit currently shows as unsigned, so it'll need a signed replacement before it can go in.

If you haven't set up SSH signing before:

git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub
git config --global commit.gpgsign true

Then add that same public key to your GitHub account a second time, under Settings → SSH and GPG keys → New SSH key, with the key type set to Signing Key — an authentication key alone won't make commits show as Verified.

Since there's only one commit here, re-signing it is:

git commit --amend --no-edit
git push --force-with-lease

You'll be force-pushing anyway, so it's a good moment to fold in the review points from my earlier comment in the same push if you want to.

Worth saying clearly: this is a process box to tick, not a reflection on the change. The fix itself tested clean on our hardware.

[vpetersson-bot]

Approach is right, and I like that redis_host() exists as a separate function rather than inlining getenv — with connect_to_redis() faked for the whole session in conftest.py there'd otherwise be no way to test this at all, and the docstring says so. Default and env var name match settings.py:570 exactly, so the channel layer and the plain client can't drift apart on the compose network.

One loose end, in the suite rather than the fix:

tests/test_app.py:2170 opens a real Redis connection with the host still hardcoded:

client: Any = _redis.Redis(
    host='redis', port=6379, db=0, decode_responses=True
)

The comment above it explains why it bypasses the conftest.py fake — it needs the actual broker, because uvicorn publishes from a separate process. That's fine today, since both sides resolve redis. But the setup your own test plan describes (REDIS_HOST=127.0.0.1, server on the host network) now splits them: the app under test publishes to 127.0.0.1 while this subscriber waits on redis. The test doesn't fail loudly either — it polls get_message until the deadline, so it reads as a timeout rather than a misconfiguration.

utils.redis_host() there instead of the literal keeps the two ends pointing at the same broker, and it's the only other real connection in the tree, so that's the whole cleanup.

Nothing else: port staying hardcoded at 6379 matches settings.py:571, which also fixes the port while reading the host from the environment, so the two stay symmetric. anthias_host_agent keeping its own REDIS_HOST = '127.0.0.1' constant is correct — it runs on the host and talks to the loopback broker, not the compose one.

Happy to approve once the test_app.py literal is pulled onto the same helper.

[vpetersson-bot]

Approving. Small, correct, and the test seam is the right one.

Verified rather than assumed:

• getenv is already imported in src/anthias_common/utils.py:10 (from os import getenv, utime), so no new import is needed and the unqualified call is consistent with the other uses in that file.
• The claim that this matches the channel layer is exact, not just similar: src/anthias_server/django_project/settings.py:570 is 'host': getenv('REDIS_HOST', 'redis') — identical expression and identical default. So a deployment that already sets REDIS_HOST for the channel layer now gets the same host here, and the compose network is unaffected.
• The hardcoded port=6379 stays in step too: the channel layer hardcodes 'port': 6379 right below (settings.py:571). Honouring the host but not the port is the symmetric choice here, not a half-fix. If REDIS_PORT ever becomes worth supporting, both sites should move together.
• The remaining hardcoded Redis hosts are correct as-is. src/anthias_host_agent/__main__.py pins 127.0.0.1 because the host agent runs outside the compose network, and tests/test_app.py is a test fixture.

Extracting redis_host() purely so it can be tested around conftest.py's session-wide connect_to_redis() fake is the right call — the comment in the test explaining why the assertion targets the helper instead of the connection is what makes that hold up for the next reader.

CI is green on all 11 checks. The branch is 10 commits behind master, so a rebase before merge would be worth it, but nothing in those commits looks like it touches this.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud