chore: add cached_height support in cache metadata #76
Conversation
- Introduced `cached_height` field in `CacheMetadata` to store height information. - Updated `TNClient` to map `Height` from responses to `cached_height`. - Modified relevant tests to validate the new `cached_height` functionality, ensuring it defaults to `None` when not set. This enhancement allows for more comprehensive cache metadata handling in the SDK.
- Introduced a new GitHub Actions CI workflow to automate testing and building processes. - The workflow includes steps for setting up Python and Go environments, installing dependencies, and running unit and integration tests. - Implemented caching for the kwil-cli build to optimize build times. - Added cleanup steps to manage Docker resources after the workflow execution. This enhancement streamlines the development process and ensures consistent testing across pull requests.
WalkthroughThis change renames the cache metadata field from timestamp to height in both the Go bindings and Python client, updating all related mappings, model fields, and tests to use the new block height-based cache metadata. It also adds a new test for the cache height mapping and introduces a CI workflow for automated testing. Changes
Sequence Diagram(s)sequenceDiagram participant User participant TNClient (Python) participant Go Bindings participant Node User->>TNClient (Python): Request data (e.g., get_records) TNClient (Python)->>Go Bindings: Call Go function Go Bindings->>Node: Query data Node-->>Go Bindings: Respond with data + cache metadata (height) Go Bindings-->>TNClient (Python): Return DataResponse (Height) TNClient (Python)-->>User: Return data + CacheMetadata (cache_height) Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File ( |
- Updated `github.com/trufnetwork/sdk-go` dependency to the latest version for improved features. - Refactored `DataResponse` structure to include `Height` field for cache metadata. - Adjusted `TNClient` methods to correctly map and handle the new `Height` field. - Updated tests to validate the new cache metadata structure and ensure compatibility with the latest SDK changes. This update enhances the cache metadata handling and ensures consistency with the latest SDK features.
- Changed the NODE_REPO_DIR environment variable in the CI workflow to use the GitHub workspace path for better compatibility and reliability during the build process. This update ensures that the CI workflow correctly references the temporary node directory, improving the overall build stability.
- Changed the step in the CI workflow from moving to copying the kwil-cli binary to the system path. This ensures the binary is correctly placed without removing the original file, enhancing build reliability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/ci.yaml (1)
63-63: Fix YAML formatting issues.The file has trailing spaces and is missing a newline at the end.
Remove trailing spaces and add a newline at the end of the file for proper YAML formatting.
Also applies to: 92-92, 95-95
tests/test_cache_support.py (1)
122-122: Apply Python style conventions for None comparisons.Use
is Noneinstead of== Nonefor None comparisons per PEP 8.Apply these diffs:
- assert result.cache.cache_height == None + assert result.cache.cache_height is None- assert result.cache.cache_height == None + assert result.cache.cache_height is None- assert result.cache.cache_height == None + assert result.cache.cache_height is NoneAlso applies to: 166-166, 212-212
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
-
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
-
.github/workflows/ci.yaml(1 hunks) -
bindings/bindings.go(5 hunks) -
go.mod(1 hunks) -
src/trufnetwork_sdk_py/client.py(2 hunks) -
tests/test_cache_support.py(3 hunks)
🧰 Additional context used
🪛 Ruff (0.12.2)
tests/test_cache_support.py
122-122: Comparison to None should be cond is None
Replace with cond is None
(E711)
166-166: Comparison to None should be cond is None
Replace with cond is None
(E711)
212-212: Comparison to None should be cond is None
Replace with cond is None
(E711)
225-225: Avoid equality comparisons to True; use metadata.hit: for truth checks
Replace with metadata.hit
(E712)
231-231: Avoid equality comparisons to False; use not metadata.hit: for false checks
Replace with not metadata.hit
(E712)
232-232: Comparison to None should be cond is None
Replace with cond is None
(E711)
src/trufnetwork_sdk_py/client.py
237-237: No explicit stacklevel keyword argument found
Set stacklevel=2
(B028)
🪛 actionlint (1.7.7)
.github/workflows/ci.yaml
18-18: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
93-93: if: condition "always() && ${{ !env.ACT }}" is always evaluated to true because extra characters are around ${{ }}
(if-cond)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 63-63: trailing spaces
(trailing-spaces)
[error] 92-92: trailing spaces
(trailing-spaces)
[error] 95-95: no new line character at the end of file
(new-line-at-end-of-file)
[error] 95-95: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Native Build on ubuntu-latest
- GitHub Check: Test Docker Distribution (Linux)
- GitHub Check: test
🔇 Additional comments (8)
go.mod (1)
11-11: LGTM: Dependency update supports cache metadata changes.The SDK dependency version update aligns with the cache metadata field renaming from timestamp to height, ensuring compatibility with the updated Go bindings.
bindings/bindings.go (4)
59-62: LGTM: Struct field rename properly implemented.The DataResponse struct correctly renames the cache metadata field from
TimestamptoHeightwith appropriate JSON tag update, maintaining the OptionalInt64 wrapper type.
288-291: LGTM: Consistent cache metadata mapping in GetRecords.The function correctly updates the cache metadata mapping to use
response.Metadata.CacheHeightinstead of the previous timestamp field, maintaining consistency with the structural changes.
371-374: LGTM: Consistent cache metadata mapping in GetFirstRecord.The function properly updates the cache metadata mapping for single record responses, ensuring consistency across all data retrieval methods.
418-421: LGTM: Consistent cache metadata mapping in GetIndex.The GetIndex function correctly applies the same cache metadata mapping pattern, completing the consistent update across all cache-aware data retrieval methods.
tests/test_cache_support.py (1)
217-232: LGTM: Comprehensive test for cache metadata mapping.The new test function thoroughly validates the
_map_cache_metadatamethod with both cache hit scenarios (including height value) and cache miss scenarios, ensuring the height-based cache metadata is correctly handled.Minor style improvement for boolean comparisons:
- assert metadata.hit == True + assert metadata.hit - assert metadata.hit == False + assert not metadata.hit - assert metadata.cache_height == None + assert metadata.cache_height is Nonesrc/trufnetwork_sdk_py/client.py (2)
117-117: LGTM: Cache metadata field rename to height-based semantics.The field rename from
cached_attocache_heightcorrectly reflects the semantic change from timestamp-based to block height-based cache metadata, maintaining consistency with the Go bindings.
210-238: LGTM: Robust cache metadata mapping with height support.The
_map_cache_metadatamethod correctly implements the new height-based mapping logic:
- Handles both attribute and dictionary access patterns
- Properly extracts height from nested
Height.IsSet/Height.Valuestructure- Maintains graceful error handling with fallback to default values
- Supports the cache hit/miss scenarios appropriately
Minor improvement for the warning call:
- warnings.warn(f"Failed to map cache metadata from Go: {e}", UserWarning) + warnings.warn(f"Failed to map cache metadata from Go: {e}", UserWarning, stacklevel=2)
Time Submission Status
|
- Added steps to the CI workflow for installing gopy and goimports, enhancing the Go development environment. - Updated the GitHub path to include the Go binaries, ensuring they are accessible during the CI process. This change improves the setup for Go-related tasks in the CI pipeline.
- Modified the CI workflow to replace the `uv sync` command with `uv pip install -e .[dev]`, ensuring that development dependencies are installed correctly in the virtual environment. This change enhances the setup for development tasks in the CI pipeline.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/ci.yaml (2)
18-20: Updateactions/setup-pythonto the maintained major version.
actions/setup-python@v4is no longer guaranteed to be supported on current GitHub-Actions runners (flagged by actionlint). Upgrade to@v5(or pin a specific minor) to avoid unexpected breakages.
100-100: Fix theifexpression syntax.
if: always() && ${{ !env.ACT }}is parsed as the literal stringalways() && ${{ !env.ACT }}and always evaluates totrue.
Remove the outer${{ }}wrapper:- if: always() && ${{ !env.ACT }} + if: always() && !env.ACT
🧹 Nitpick comments (2)
.github/workflows/ci.yaml (2)
37-37: YAML-lint errors: trailing whitespace and missing newline.Lines 37, 70, 99 and 102 contain trailing spaces, and the file lacks a terminating newline (line 102). Clean these up to keep the workflow YAML valid and avoid CI lint failures.
Also applies to: 70-70, 99-102
80-85: Consider caching Python dependencies for quicker CI cycles.
uv pip install -e .[dev]re-downloads and resolves the full dependency tree on every run. Adding anactions/cachestep keyed onpoetry.lock/requirements.txt(oruv.lock) and Python version can cut several minutes off the workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
-
.github/workflows/ci.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yaml
18-18: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
100-100: if: condition "always() && ${{ !env.ACT }}" is always evaluated to true because extra characters are around ${{ }}
(if-cond)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 102-102: no new line character at the end of file
(new-line-at-end-of-file)
[error] 102-102: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test Docker Distribution (Linux)
- GitHub Check: test
- Updated the CI workflow to rename the "Run unit tests" step to "Run tests" and removed the separate integration tests step, allowing all tests to be executed in a single command. - Adjusted the cleanup condition to improve clarity. This change streamlines the testing process in the CI pipeline.
- Modified tests in `test_cache_support.py` to validate the new `CacheAwareResponse` structure when using cache-aware signatures. - Updated assertions to check for cache metadata, including cache hit status and cache height. - Adjusted the `test_map_cache_metadata_with_height` function to utilize the `client` instance for mapping cache metadata. This update ensures that the tests accurately reflect the latest changes in cache handling and response structures.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci.yaml (1)
18-20: Update GitHub Action to supported version.The
actions/setup-python@v4action version is outdated and no longer supported on current GitHub Actions runners.Apply this diff to use a supported version:
- - name: Set up Python - uses: actions/setup-python@v4 - with: + - name: Set up Python + uses: actions/setup-python@v5 + with:
🧹 Nitpick comments (6)
.github/workflows/ci.yaml (1)
93-97: Fix formatting issues.Multiple formatting issues detected by static analysis tools.
Apply this diff to fix trailing spaces and add missing newline:
- name: Cleanup - # not act + # not act if: always() && !env.ACT run: | - docker system prune -af + docker system prune -af +tests/test_cache_support.py (5)
79-84: Update comparison style for better Python practices.The logic correctly validates the cache miss scenario with the new
cache_heightfield, but the comparison style should be improved.Apply this diff to improve comparison style:
# Should return CacheAwareResponse structure, because we are using the cache-aware signature assert isinstance(result, CacheAwareResponse) assert result.data is not None assert result.cache is not None - assert result.cache.hit == False - assert result.cache.cache_height == None + assert not result.cache.hit + assert result.cache.cache_height is None
123-123: Improve None comparison style.The field update from
cached_attocache_heightcorrectly aligns with the PR objective.Apply this diff to use proper None comparison:
- assert result.cache.cache_height == None + assert result.cache.cache_height is None
167-167: Improve None comparison style.The field update correctly reflects the new cache metadata structure.
Apply this diff:
- assert result.cache.cache_height == None + assert result.cache.cache_height is None
213-213: Improve None comparison style.Consistent with other test updates for the new cache metadata structure.
Apply this diff:
- assert result.cache.cache_height == None + assert result.cache.cache_height is None
218-233: Excellent test coverage for cache metadata mapping functionality.This new test directly validates the
_map_cache_metadatamethod and covers both cache hit/miss scenarios with height values, which is essential for the new cache metadata structure.Apply this diff to improve comparison style throughout the test:
def test_map_cache_metadata_with_height(client): """Test mapping cache metadata with height""" # Test cache hit with height mock_response = { 'CacheHit': True, 'Height': {'IsSet': True, 'Value': 123456} } metadata = client._map_cache_metadata(mock_response) # type: ignore - assert metadata.hit == True + assert metadata.hit assert metadata.cache_height == 123456 # Test miss case mock_miss = {'CacheHit': False} metadata = client._map_cache_metadata(mock_miss) # type: ignore - assert metadata.hit == False - assert metadata.cache_height == None + assert not metadata.hit + assert metadata.cache_height is None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.yaml(1 hunks)tests/test_cache_support.py(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/test_cache_support.py (2)
src/trufnetwork_sdk_py/client.py (2)
CacheAwareResponse(120-124)_map_cache_metadata(210-238)tests/test_tnclient.py (1)
client(12-19)
🪛 actionlint (1.7.7)
.github/workflows/ci.yaml
18-18: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: no new line character at the end of file
(new-line-at-end-of-file)
[error] 97-97: trailing spaces
(trailing-spaces)
🪛 Ruff (0.12.2)
tests/test_cache_support.py
83-83: Avoid equality comparisons to False; use not result.cache.hit: for false checks
Replace with not result.cache.hit
(E712)
84-84: Comparison to None should be cond is None
Replace with cond is None
(E711)
123-123: Comparison to None should be cond is None
Replace with cond is None
(E711)
167-167: Comparison to None should be cond is None
Replace with cond is None
(E711)
213-213: Comparison to None should be cond is None
Replace with cond is None
(E711)
226-226: Avoid equality comparisons to True; use metadata.hit: for truth checks
Replace with metadata.hit
(E712)
232-232: Avoid equality comparisons to False; use not metadata.hit: for false checks
Replace with not metadata.hit
(E712)
233-233: Comparison to None should be cond is None
Replace with cond is None
(E711)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: Test Docker Distribution (Linux)
🔇 Additional comments (2)
.github/workflows/ci.yaml (2)
58-75: Well-designed caching and verification strategy.The build caching using commit hash as the key and the smoke test verification demonstrate good CI practices for managing external dependencies.
25-29: No changes needed: Go 1.24 is available and stable.As of July 8, 2025, Go 1.24.5 is the latest patch release and using
with: go-version: '1.24' check-latest: trueensures your workflow will pick it up.
Your setup is correct.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
38-38: Clean up trailing whitespace and add a newline at EOFYAML-lint flags lines 38, 71, 95 and the file end for trailing spaces / missing newline. While harmless to runners, it will keep the linter red and can hinder future diffs.
… … … +Also applies to: 71-71, 95-98
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/ci.yaml
17-17: step must run script with "run" section or run action with "uses" section
(syntax-check)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 38-38: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 98-98: no new line character at the end of file
(new-line-at-end-of-file)
[error] 98-98: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Docker Distribution (Linux)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yaml (1)
37-37: Clean up trailing whitespace and add a final newline
yamllintflags lines 37, 70, 94, 97 for trailing spaces and the file for missing
a terminating newline. While harmless at runtime, these cause lint failures in
strict CI setups.- shell: bash␠ + shell: bash … - task build␠ + task build … - # not act ␠ + # not act … - docker system prune -af ␠ + docker system prune -af +\nAlso applies to: 70-70, 94-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[error] 37-37: trailing spaces
(trailing-spaces)
[error] 70-70: trailing spaces
(trailing-spaces)
[error] 94-94: trailing spaces
(trailing-spaces)
[error] 97-97: no new line character at the end of file
(new-line-at-end-of-file)
[error] 97-97: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: Test Docker Distribution (Linux)
🔇 Additional comments (1)
.github/workflows/ci.yaml (1)
17-21: 👍 Updated toactions/setup-python@v5Switching to the supported major version resolves the previously-reported
runner-compatibility warning.
cached_heightfield inCacheMetadatato store height information.TNClientto mapHeightfrom responses tocached_height.cached_heightfunctionality, ensuring it defaults toNonewhen not set.This enhancement allows for more comprehensive cache metadata handling in the SDK.
Summary by CodeRabbit