-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(docs): Remove outdated network endpoints and add endpoint check action #177
base: main
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@MalteHerrmann has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new GitHub Actions workflow named Changes
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for dancing-hummingbird-242a98 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🧹 Outside diff range and nitpick comments (11)
scripts/check_endpoints/test_queries.py (1)
1-7
: Enhance module documentation and test organization.The module docstring could be more descriptive. Consider adding:
- Purpose of these tests
- Prerequisites for running them
- Expected test environment setup
""" -This file contains the tests for the endpoint queries. +This file contains integration tests for network endpoint availability checks. + +Tests verify the ability to query different types of endpoints: +- Ethereum JSON-RPC +- Cosmos REST +- Tendermint RPC + +Prerequisites: +- Network access to test endpoints +- Required dependencies installed """Consider adding pytest fixtures for common setup and teardown operations.
scripts/check_endpoints/test_endpoints.py (1)
1-5
: Enhance module docstring with more details.Consider expanding the docstring to include:
- Description of test cases
- Expected behavior
- Any test data dependencies
""" -This file contains the tests for the logic -that is extracting the available endpoints -from the Markdown file. +Unit tests for endpoint extraction functionality from Markdown files. + +Test cases: +- test_get_endpoint_from_line_pass: Validates parsing of individual markdown table rows +- test_get_endpoints_pass: Ensures successful extraction of mainnet and testnet endpoints + +Dependencies: +- Requires testdata/networks.mdx file for endpoint validation """scripts/check_endpoints/main.py (2)
1-5
: Enhance module docstring with more details.Consider adding information about:
- The types of endpoints being checked
- The specific queries being performed
- The expected outcomes and error handling
""" This tool checks the list of available endpoints for their functionality. Any non-responsive endpoints are being flagged to be either discussed with the corresponding partners or removed. + + The tool performs health checks on both mainnet and testnet endpoints + by sending specific API queries. Endpoints that fail to respond or + return errors are flagged for review. The script exits with an error + if any endpoints are non-responsive. """
14-17
: Add docstring to print_output function.The function would benefit from documentation explaining the meaning of the status indicators.
def print_output(endpoint: Endpoint, ok: Union[bool, None]): + """ + Print the endpoint status with visual indicators. + + Args: + endpoint: The endpoint object containing address and type + ok: Status of the endpoint check + - True: endpoint is responsive (✅) + - False: endpoint is non-responsive (❌) + - None: status unknown (❓) + """ ok_emoji = "❓" if ok is None else ("✅" if ok else "❌") print(f" {ok_emoji} - {endpoint.address}")scripts/check_endpoints/queries.py (2)
1-10
: Enhance module documentation and organize importsConsider improving the module documentation and organizing imports according to PEP 8:
""" -This file contains the required logic to -call a simple query on a given endpoint -based on the type of endpoint. +Contains endpoint querying logic for different API types: +- Ethereum JSON-RPC endpoints +- Cosmos REST endpoints +- Tendermint RPC endpoints + +Each query function performs a basic health check by requesting +endpoint-specific data. """ -from endpoints import Endpoint -import requests from typing import Union +import requests + +from endpoints import Endpoint
26-68
: Consider reducing code duplication across query functionsThe three query functions share similar patterns for making requests, handling errors, and validating responses. Consider extracting these common patterns into a base helper function:
def _make_request( method: str, url: str, category: str, json_data: Optional[dict] = None, response_validator: Callable[[dict], bool] = lambda x: True ) -> bool: """ Make a request to an endpoint and validate the response. Args: method: HTTP method ("GET" or "POST") url: The endpoint URL category: Endpoint category for error messages json_data: Optional JSON payload for POST requests response_validator: Function to validate response content Returns: bool: True if the request was successful and validation passed """ try: response = requests.request( method, url, json=json_data, timeout=10, verify=True ) if response.status_code != requests.codes.OK: print(f"{category} endpoint {url} returned status {response.status_code}") return False return response_validator(response.json()) except requests.Timeout: print(f"{category} endpoint {url} timed out") return False except requests.SSLError: print(f"{category} endpoint {url} failed SSL verification") return False except Exception as e: print(f"failed to query {category} endpoint {url}: {e}") return FalseThis would simplify the query functions to:
def query_json_rpc(address: str) -> bool: return _make_request( "POST", address, "JSON-RPC", json_data={"jsonrpc": "2.0", "method": "eth_blockNumber", "params": [], "id": 83}, response_validator=lambda data: all(key in data for key in ["jsonrpc", "id", "result"]) ) def query_cosmos_rest(address: str) -> bool: return _make_request( "GET", urljoin(address, "cosmos/base/tendermint/v1beta1/blocks/latest"), "Cosmos REST", response_validator=lambda data: "block" in data ) def query_tendermint_rpc(address: str) -> bool: return _make_request( "GET", urljoin(address, "block"), "Tendermint RPC", response_validator=lambda data: "result" in data and "block" in data["result"] )scripts/check_endpoints/endpoints.py (1)
1-4
: Enhance the module docstring with format details.Consider expanding the docstring to include:
- The expected Markdown table format
- Example of a valid endpoint entry
- Description of the sections (Mainnet/Testnet) being parsed
Example enhancement:
""" This file contains the logic to extract the available endpoints from the Markdown file of the docs. + + The Markdown file should contain sections for Mainnet and Testnet, + with endpoints listed in tables of the following format: + + | `endpoint-url` | `type1` `type2` | [Provider Name] | """docs/develop/api/networks.mdx (1)
26-29
: Consider adding endpoint reliability information.While the new Lava Network endpoints are well-documented, users would benefit from additional information about:
- Endpoint uptime guarantees
- Rate limiting details
- Health monitoring status
This information would help users make informed decisions about which endpoints to use for their applications.
scripts/check_endpoints/testdata/networks.mdx (3)
20-67
: Consider standardizing URL formats and adding security notes.While the endpoint list is comprehensive, consider the following improvements:
- Some URLs end with trailing slashes while others don't (e.g., line 42 vs 43)
- Some URLs explicitly show port
:443
while others don't- Missing security recommendations for users regarding endpoint trust and SSL verification
Consider adding a security note at the beginning of the endpoints section:
+:::caution +When connecting to public endpoints: +- Always verify SSL certificates +- Be cautious with sensitive data +- Consider rate limits and fair usage +:::
96-104
: Improve readability with minor language refinements.Consider these language improvements:
-Lava is an open source protocol +Lava is an open-source protocol -in a similar way to how Uber +similarly to how Uber -ensured by Lava protocol +ensured by the Lava protocol -Lava is the providing a highly reliable +Lava is providing highly reliable🧰 Tools
🪛 LanguageTool
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n-Demand Services ### Lava Lava is an open source protocol that serves as a p2p market fo...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~102-~102: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...va pairs consumers and providers of RPC in a similar way to how Uber connects passengers and dri...(IN_A_X_MANNER)
[uncategorized] ~102-~102: You might be missing the article “the” here.
Context: ...data reliability & accuracy, ensured by Lava protocol and the crypto-economic framew...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
1-104
: Consider adding endpoint health monitoring information.Given that this PR introduces endpoint checking functionality, consider adding a section about:
- How endpoints are monitored for availability
- What defines a "healthy" endpoint
- How often endpoints are checked
- What happens when an endpoint becomes unavailable
Would you like help drafting this additional documentation section?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n-Demand Services ### Lava Lava is an open source protocol that serves as a p2p market fo...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~102-~102: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...va pairs consumers and providers of RPC in a similar way to how Uber connects passengers and dri...(IN_A_X_MANNER)
[uncategorized] ~102-~102: You might be missing the article “the” here.
Context: ...data reliability & accuracy, ensured by Lava protocol and the crypto-economic framew...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (11)
- .github/workflows/check-endpoints.yml (1 hunks)
- docs/develop/api/networks.mdx (1 hunks)
- requirements.txt (1 hunks)
- scripts/.gitignore (1 hunks)
- scripts/check_endpoints/init.py (1 hunks)
- scripts/check_endpoints/endpoints.py (1 hunks)
- scripts/check_endpoints/main.py (1 hunks)
- scripts/check_endpoints/queries.py (1 hunks)
- scripts/check_endpoints/test_endpoints.py (1 hunks)
- scripts/check_endpoints/test_queries.py (1 hunks)
- scripts/check_endpoints/testdata/networks.mdx (1 hunks)
✅ Files skipped from review due to trivial changes (3)
- requirements.txt
- scripts/.gitignore
- scripts/check_endpoints/init.py
🧰 Additional context used
🪛 LanguageTool
scripts/check_endpoints/testdata/networks.mdx
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n-Demand Services ### Lava Lava is an open source protocol that serves as a p2p market fo...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~102-~102: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...va pairs consumers and providers of RPC in a similar way to how Uber connects passengers and dri...(IN_A_X_MANNER)
[uncategorized] ~102-~102: You might be missing the article “the” here.
Context: ...data reliability & accuracy, ensured by Lava protocol and the crypto-economic framew...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (6)
.github/workflows/check-endpoints.yml (1)
24-25
: Verify the endpoint check script path.Let's verify that the script path exists in the repository.
✅ Verification successful
Script exists and is properly structured for execution
The script at
scripts/check_endpoints/main.py
exists and is well-structured with:
- A main execution block (
if __name__ == "__main__"
)- Core functionality in
check_endpoints()
function- Clear error handling that raises exceptions on failures
- Proper imports and type hints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the endpoint check script # Test: Check if the script exists if ! fd "main.py" "scripts/check_endpoints/"; then echo "Error: scripts/check_endpoints/main.py not found" exit 1 fi # Test: Check if the script is executable if ! rg -l "def main" "scripts/check_endpoints/main.py"; then echo "Warning: main.py might not contain a main function" fiLength of output: 231
Script:
#!/bin/bash # Let's check the content of the script to understand its structure cat scripts/check_endpoints/main.pyLength of output: 1633
scripts/check_endpoints/test_endpoints.py (1)
22-22
: Verify test data file existence and content.Let's verify the existence and structure of the test data file.
✅ Verification successful
Test data file exists and contains valid endpoint data
The test data file at
scripts/check_endpoints/testdata/networks.mdx
exists and contains properly formatted endpoint data with both mainnet and testnet sections in a markdown table structure. The file includes comprehensive endpoint information including addresses, categories, maintainers, and node types.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if test data file exists and contains valid endpoint data test_file="scripts/check_endpoints/testdata/networks.mdx" if [ -f "$test_file" ]; then echo "Test file exists. Checking content..." # Check for expected markdown table structure grep -A 5 "^|.*|.*|.*|.*|$" "$test_file" else echo "Warning: Test data file not found at $test_file" # Search for possible locations fd --type f "networks.mdx" fiLength of output: 9182
scripts/check_endpoints/main.py (1)
7-9
: Verify the implementation of imported local modules.Let's ensure the imported modules exist and contain the expected implementations.
✅ Verification successful
All imported modules are properly implemented and contain the expected functionality.
The verification confirms:
endpoints.py
exists and contains:
Endpoint
dataclass with required fields (address, category, provider)get_endpoints
function that parses endpoints from markdown filesqueries.py
exists and contains:
query_endpoint
function that handles different endpoint types- Implementation for all endpoint category queries
Both modules also include corresponding test files validating their functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for the existence and content of imported modules echo "Checking for endpoints.py implementation..." fd --type f "endpoints.py" --exec cat {} echo -e "\nChecking for queries.py implementation..." fd --type f "queries.py" --exec cat {} echo -e "\nVerifying Endpoint class usage..." ast-grep --pattern 'class Endpoint: $$$'Length of output: 6589
docs/develop/api/networks.mdx (1)
Line range hint
1-100
: LGTM! Documentation structure is clear and well-organized.The documentation provides a comprehensive overview of available endpoints with clear categorization and maintainer information. The removal of outdated endpoints and addition of new ones improves the reliability of the documentation.
Let's verify that all listed endpoints are active:
scripts/check_endpoints/testdata/networks.mdx (2)
1-19
: LGTM! Well-structured introduction with helpful resources.The introduction provides clear navigation and useful external resources for users.
69-94
: LGTM! Testnet endpoints are well-documented.The testnet endpoints section maintains consistency with the mainnet format and provides a good selection of endpoints.
elif line[0] != "|": | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this already covered by line 50?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no it's not because all lines with #
end up here, but I only wanted to parse the table contents, which start with the |
from typing import Union | ||
|
||
|
||
def query_endpoint(ep: Endpoint) -> Union[bool, None]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about websocket and Cosmos gRPC.
nit: not part of this PR, but it would be nice to rename Tendermint to Comet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will create a follow-up issue to address the CometBFT names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Nice job @MalteHerrmann, just added a couple of quesitons.
This PR adds a script to check the integrity of a selection of listed publicly available endpoints.
Based on the checked results, I have removed a bunch of outdated entries from the mainnet and testnet lists.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
requirements.txt
file to include new dependencies for improved functionality.Tests