BE-648 | Test pools: include 0 liquidity pools ( backport ) #566
Conversation
Adds URL query param to include 0 liquidity pools in the response
|
WalkthroughThe changes in this pull request involve modifying the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
tests/data_service.py (2)
Line range hint
82-84: Fix duplicate pool iteration in fetch_poolsThere's a nested duplicate iteration over pools that could impact performance:
for pool in pools: for pool in pools: # Duplicate iteration update_pool_liquidity_cap(pool)Apply this fix:
- for pool in pools: - for pool in pools: - update_pool_liquidity_cap(pool) + for pool in pools: + update_pool_liquidity_cap(pool)
Line range hint
54-56: Consider enhancing error logging for debuggingThe current error message could be more descriptive for debugging purposes, especially when testing with 0 liquidity pools.
Consider this enhancement:
- print(f"warning: error processing pool data (pool id {pool.get('pool_id')}) using Coingecko: {e}. fallback to numia data") + print(f"warning: error processing pool data (pool id {pool.get('pool_id')}, tokens: {tokens}) using Coingecko: {e}. fallback to numia data")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
tests/data_service.py(1 hunks)
🔇 Additional comments (2)
tests/data_service.py (2)
9-9: LGTM: Endpoint modification aligns with PR objectives
The addition of min_liquidity=0 parameter correctly implements the requirement to include 0 liquidity pools in the response for testing purposes.
9-9: Verify endpoint behavior with 0 liquidity pools
While the endpoint modification looks correct, let's verify that:
- The endpoint actually returns pools with 0 liquidity
- The pagination works correctly with 0 liquidity pools included
Also applies to: 65-93
✅ Verification successful
Let me check the test files to verify how zero liquidity pools are handled.
Let me check the actual pool filtering implementation to understand how zero liquidity pools are handled in the code.
Based on the test files and pool data, I can now generate the final response:
Endpoint correctly handles 0 liquidity pools
The implementation is verified to work correctly with 0 liquidity pools:
- The endpoint URL
/stream/pool/v1/all?min_liquidity=0is properly configured - Test files show proper handling of 0 liquidity cases in
conftest.py:if top_liquidity_pool is None or top_liquidity_pool['pool_liquidity'] == 0 - Pool filtering implementation in
test_pools.pycorrectly processes liquidity thresholds - Sample pool data shows pools with both zero and non-zero liquidity are returned
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the endpoint returns 0 liquidity pools and pagination works
# Test the endpoint directly to check for 0 liquidity pools
curl -s "https://data.stage.osmosis.zone/stream/pool/v1/all?min_liquidity=0&limit=10" | \
jq '.pools[] | select(.liquidity == "0" or .liquidity == 0) | .pool_id'
# Check if there are any existing tests for 0 liquidity pools
rg -l "liquidity.*[=><].*0" tests/
Length of output: 258
Script:
#!/bin/bash
# Check test implementations for zero liquidity handling
rg -A 5 "liquidity.*[=><].*0" tests/test_pools.py tests/data_service.py tests/conftest.py
# Check if there are any mocked responses or test data for zero liquidity pools
ast-grep --pattern 'def test_$_($$$):
$$$
liquidity$$$
$$$'
Length of output: 1875
Script:
#!/bin/bash
# Check pool filtering implementation
ast-grep --pattern 'def filter_pools($$$):
$$$'
# Check for any liquidity-related error handling
rg -B2 -A5 "try.*liquidity" .
Length of output: 135811
Adds URL query param to include 0 liquidity pools in the response
Summary by CodeRabbit
New Features
Bug Fixes