Prefer ZooKeeper configuration over static config

[mithunbharadwaj]

Summary by CodeRabbit

• New Features

  • ZooKeeper configuration overrides applied at startup—values from ZooKeeper take precedence over static config; service continues with static values if ZooKeeper or nodes are unavailable.

• Documentation

  • Added guide describing the override behavior, expected JSON payload structure, handling semantics for missing/unavailable nodes, and example settings to enable the feature.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ef3afa1a-fd79-4f75-b21d-69adc715c530

📥 Commits

Reviewing files that changed from the base of the PR and between 7ebc29f and 53a2c43.

📒 Files selected for processing (1)

• asabiris/config_zookeeper.py

---

📝 Walkthrough

Walkthrough

Adds ZooKeeper-backed configuration overrides layered onto the existing static config. A new module loads JSON from a configured znode, stringifies and merges values into asab.Config at bootstrap, and asabiris/app.py applies these overrides after ZooKeeper setup and before service instantiation.

Changes

Cohort / File(s)	Summary
Documentation README.md	Added "ZooKeeper Configuration Overrides" section describing overlay semantics, handling of missing/unavailable nodes, example [zookeeper] and [config_zookeeper] settings, and expected JSON payload format mapping top-level keys to config sections.
Config Override Module asabiris/config_zookeeper.py	New module adding _stringify_config_value(), resolve_config_zookeeper_path(), load_config_overrides(), apply_config_overrides(), and apply_zookeeper_config_overrides() to read JSON from a znode, validate/stringify values, and shallow-merge overrides into asab.Config with fail-soft semantics.
Bootstrap Integration asabiris/app.py	Imported and invoked apply_zookeeper_config_overrides(app) after creating the ZooKeeper container (or None) and before instantiating LibraryService, ensuring overrides are applied prior to service creation.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant ZK as ZooKeeper
    participant Config as asab.Config
    participant Service as LibraryService

    App->>App: Load static config
    App->>ZK: Create ZooKeeperContainer (if configured)
    App->>App: resolve znode path from `[config_zookeeper]`
    App->>ZK: read znode content
    alt node present & valid JSON
        ZK-->>App: JSON payload
        App->>App: stringify & validate values
        App->>Config: apply overrides (merge into sections)
        Config-->>App: config updated
        App-->>Service: instantiate with finalized config
    else node missing/unavailable or error
        ZK-->>App: empty/error
        App-->>Service: instantiate with static config
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hop through keys where z-nodes hide,

I shape their JSON, tuck defaults aside.
When overrides whisper from the tree,
I stitch them in for services to see.
No node? I leave the static bride.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main objective of the pull request—implementing a ZooKeeper configuration overlay mechanism that prioritizes ZooKeeper-stored settings over static configuration.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch Feature/zookeeper-config-overlay

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

README.md (1)

46-48: Document the supported url alternative in [config_zookeeper].

The implementation accepts both path and url as inputs for resolving the znode path, but this section only shows path. Adding the url variant here would keep operator guidance aligned with runtime behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 46 - 48, Update the README's [config_zookeeper]
example to document the supported alternative key by showing both path and url
variants; mention that the implementation accepts either the path key (path) or
the url key (url) for resolving the znode path so operators know both are valid
and how to use them when configuring the service.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@asabiris/config_zookeeper.py`:
- Around line 100-110: The code decodes byte znode data to payload_raw and then
calls json.loads even when payload_raw is an empty string, causing a parse
warning; modify the branch that handles bytes (where data is decoded into
payload_raw) to check if payload_raw is empty (e.g., payload_raw == "" or
payload_raw.strip() == "") and if so log the existing L.info message and return
{} immediately (same behavior as the earlier data is None branch) before calling
json.loads; adjust the logic around payload_raw, payload =
json.loads(payload_raw), and the subsequent isinstance(payload, dict) check
accordingly.
- Around line 95-100: Replace the two-step exists()/get() in
load_config_overrides() with a single guarded get() call and handle Kazoo
exceptions locally: wrap zk_client.get(path) in a try/except that catches
kazoo.exceptions.NoNodeError and kazoo.exceptions.KazooException, log a
descriptive message via L (e.g., info/warn) and return {} on those errors so
ZooKeeper race conditions or transient failures don’t propagate; remove the
prior exists() check and ensure only
configparser.Error/ValueError/json.JSONDecodeError are left to be handled by the
outer parser logic.

---

Nitpick comments:
In `@README.md`:
- Around line 46-48: Update the README's [config_zookeeper] example to document
the supported alternative key by showing both path and url variants; mention
that the implementation accepts either the path key (path) or the url key (url)
for resolving the znode path so operators know both are valid and how to use
them when configuring the service.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cc947c6c-0183-412e-8a83-8c48db1e2e77

📥 Commits

Reviewing files that changed from the base of the PR and between 8f74d5b and d6c4252.

📒 Files selected for processing (3)

• README.md
• asabiris/app.py
• asabiris/config_zookeeper.py