update pool-config: document all supported config keys in README, clarify inheritance and defaults usage #23
Conversation
…rify inheritance and defaults usage - Added a section in the README.md detailing all keys supported for pool configuration tuning using `rq/create-client`. - Clarifies the use of `:inherit-defaults?` flag to opt-out of default pooling settings. - Shows examples for: custom pool config, using Jedis defaults with `:skip`, and totally custom config without fallback defaults. - Rewords some sentences for clarity and documents exact parameter names for easier mapping. - No functional code changes; documentation only. fixed: #22 Signed-off-by: Avelino <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (3)
Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds configurable connection pool settings to the Redis client, allowing users to customize GenericObjectPoolConfig parameters when creating a client. Previously, the library relied on Jedis' default pool configuration without any customization options.
- Added pool configuration support with sensible defaults (max-total: 128, min-idle: 16, health checks enabled)
- Implemented flexible configuration resolution supporting maps, instances, or
:skipkeyword - Updated
create-clientto accept an options map as the second parameter (backward compatible)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/com/moclojer/rq.clj | Core implementation: added pool configuration data structures, resolution logic, and updated create-client to support options map |
| test/com/moclojer/rq_test.clj | Comprehensive tests for pool configuration resolution and options map usage |
| README.md | Documentation for pool configuration feature with usage examples and supported keys |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **unarquive-channel!**: This function unarchives a channel, making it active again. It is useful for reactivating channels that were previously archived. | ||
|
|
||
| ```clojure | ||
| (rq-pubsub/unarquive-channel! client "my-channel") |
There was a problem hiding this comment.
Corrected spelling of 'unarquive' to 'unarchive'.
Co-authored-by: Copilot <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/com/moclojer/rq.clj (1)
138-144: Restore global pool reuse whenref?is true.When
ref?is true and*redis-pool*already holds a pool, we should hand that ref back without creating (and leaking) anotherJedisPooled. The current logic always builds a new pool and wraps it in a fresh atom, so callers lose the shared ref and we allocate an unused connection.([url ref? pool-config] - (let [pool (create-pool url pool-config) - store? (boolean ref?)] - (if (and store? (not @*redis-pool*)) - (dosync - (ref-set *redis-pool* pool) - *redis-pool*) - (atom pool)))) + (let [store? (boolean ref?)] + (if (and store? @*redis-pool*) + *redis-pool* + (let [pool (create-pool url pool-config)] + (if store? + (dosync + (ref-set *redis-pool* pool) + *redis-pool*) + (atom pool))))))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)src/com/moclojer/rq.clj(2 hunks)test/com/moclojer/rq_test.clj(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-09-05T10:25:29.194Z
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 0
File: :0-0
Timestamp: 2024-09-05T10:25:29.194Z
Learning: The `src/com/moclojer/rq/pubsub.clj` file contains the following functions: `publish!`, `group-handlers-by-channel`, `create-listener`, `unarquive-channel!`, `pack-workers-channels`, and `subscribe!`.
Applied to files:
README.md
📚 Learning: 2024-07-03T18:17:54.022Z
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 5
File: test/com/moclojer/rq/pubsub_test.clj:8-27
Timestamp: 2024-07-03T18:17:54.022Z
Learning: In the `pubsub-test` function, the user prefers not to include `close-client` in the `finally` block, arguing that the client will be down automatically if there's a problem.
Applied to files:
README.md
📚 Learning: 2024-10-08T19:16:22.185Z
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 6
File: src/com/moclojer/rq/queue.clj:10-13
Timestamp: 2024-10-08T19:16:22.185Z
Learning: In the `push!` function of `src/com/moclojer/rq/queue.clj`, options are passed directly as keyword arguments, not within a map.
Applied to files:
src/com/moclojer/rq.clj
| {:pool-config {:max-total 64 | ||
| :max-wait-ms 2_000 | ||
| :test-on-borrow false}})) |
There was a problem hiding this comment.
Fix the numeric literal example.
Clojure’s reader still rejects digit separators like 2_000, so copying this snippet into the REPL will fail. Please swap the literal for a form that actually reads (e.g. 2000). (ask.clojure.org)
🤖 Prompt for AI Agents
In README.md around lines 177 to 179, the numeric literal example uses a digit
separator (`2_000`) which the Clojure reader rejects; replace the literal with a
valid one (for example `2000`) so the snippet reads and evaluates in the REPL;
update the pool-config example accordingly and ensure no other numeric literals
in the surrounding example use underscore separators.
There was a problem hiding this comment.
@avelino, here's the corrected code for the pool configuration example:
(def *redis-pool*
(rq/create-client "redis://localhost:6379/0"
{:pool-config {:max-total 64
:max-wait-ms 2000
:test-on-borrow false}}))The numeric literal has been changed from 2_000 to 2000 so it will properly read and evaluate in the REPL. You can now commit this change.
🧠 Learnings used
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 0
File: :0-0
Timestamp: 2024-09-05T10:25:29.194Z
Learning: The `src/com/moclojer/rq/pubsub.clj` file contains the following functions: `publish!`, `group-handlers-by-channel`, `create-listener`, `unarquive-channel!`, `pack-workers-channels`, and `subscribe!`.
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 0
File: :0-0
Timestamp: 2024-10-18T16:07:52.609Z
Learning: The `src/com/moclojer/rq/pubsub.clj` file contains the following functions: `publish!`, `group-handlers-by-channel`, `create-listener`, `unarquive-channel!`, `pack-workers-channels`, and `subscribe!`.
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 6
File: src/com/moclojer/rq/queue.clj:10-13
Timestamp: 2024-10-08T19:16:22.185Z
Learning: In the `push!` function of `src/com/moclojer/rq/queue.clj`, options are passed directly as keyword arguments, not within a map.
Learnt from: J0sueTM
Repo: moclojer/clj-rq PR: 6
File: src/com/moclojer/rq/queue.clj:10-13
Timestamp: 2024-07-05T20:37:20.365Z
Learning: In the `push!` function of `src/com/moclojer/rq/queue.clj`, options are passed directly as keyword arguments, not within a map.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Caution Docstrings generation - FAILED No docstrings were generated. |
rq/create-client.:inherit-defaults?flag to opt-out of default pooling settings.:skip, and totally custom config without fallback defaults.fixed: #22
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests