(DAL) Simplify api endpoint #1865
Conversation
WalkthroughWalkthroughThe changes involve a significant restructuring of API endpoint paths by removing versioning prefixes across various files in the project. This simplification allows direct access to endpoints without needing to include versioning in the URL, potentially enhancing usability for clients and streamlining the API's routing logic. 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![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, codebase verification and nitpick comments (1)
node/pkg/dal/tests/main_test.go (1)
115-115: Update Route Definitions to Match the New Base PathThe base path for the API group has been updated to remove versioning. However, several files still use the old base path (
/api/v1). Please update the following files to use the new base path (""):
node/pkg/admin/tests/main_test.gonode/pkg/boot/tests/main_test.gonode/pkg/boot/boot.gonode/pkg/admin/admin.godelegator/main.goapi/tests/apierr_test.goapi/tests/proxy_test.goapi/tests/reporter_test.goapi/tests/listener_test.goapi/tests/chain_test.goapi/main.goapi/tests/vrf_test.goapi/tests/service_test.goPlease ensure that all route definitions in these files are updated to match the new base path.
Analysis chain
LGTM! But verify the route definitions.
The base path for the API group has been updated to remove versioning, simplifying the endpoint structure.
However, ensure that all route definitions in the codebase are updated to match the new structure.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all route definitions use the updated base path. # Test: Search for route definitions. Expect: Only occurrences of the new base path. rg --type go -A 5 $'app.Group'Length of output: 4404
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
sentinel/pkg/checker/health/baobab_healthcheck.jsonis excluded by!**/*.jsonsentinel/pkg/checker/health/cypress_healthcheck.jsonis excluded by!**/*.json
Files selected for processing (7)
- node/pkg/dal/api/route.go (1 hunks)
- node/pkg/dal/app.go (1 hunks)
- node/pkg/dal/tests/api_test.go (4 hunks)
- node/pkg/dal/tests/collector_test.go (1 hunks)
- node/pkg/dal/tests/main_test.go (1 hunks)
- node/pkg/dal/utils/initializer/initializer.go (1 hunks)
- node/pkg/dal/utils/stats/stats.go (1 hunks)
Additional comments not posted (9)
node/pkg/dal/api/route.go (1)
9-9: Ensure route grouping change does not break existing functionality.The route grouping has been changed from
"/dal"to"", which means routes are now directly accessible. Verify that this change does not break any existing functionality or client integrations.node/pkg/dal/app.go (1)
43-43: Ensure route grouping change does not break existing functionality.The route grouping has been changed from
"/api/v1"to"", which means routes are now directly accessible. Verify that this change does not break any existing functionality or client integrations.node/pkg/dal/utils/stats/stats.go (1)
81-81: Ensure condition change does not affect statistics gathering.The condition for bypassing further processing has been changed from
"/api/v1"to"/". Verify that this change does not affect the gathering of statistics for other endpoints.node/pkg/dal/tests/collector_test.go (1)
59-59: LGTM! But verify the WebSocket endpoint usage.The WebSocket endpoint URL has been updated to match the new simplified structure.
However, ensure that all WebSocket connections in the codebase use the updated endpoint.
Verification successful
WebSocket endpoint usage verified.
The WebSocket connections in the codebase use service-specific endpoints, and the test files correctly use the updated endpoint
ws://localhost:8090/ws.
node/pkg/dal/tests/collector_test.gonode/pkg/dal/tests/api_test.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all WebSocket connections use the updated endpoint. # Test: Search for WebSocket connections. Expect: Only occurrences of the new endpoint. rg --type go -A 5 $'wss.NewWebsocketHelper'Length of output: 13995
Script:
#!/bin/bash # Description: Search for the definition or assignment of the URL variable to verify it uses the updated endpoint. # Test: Search for the definition or assignment of the URL variable. rg --type go 'URL\s*[:=]'Length of output: 3118
node/pkg/dal/utils/initializer/initializer.go (1)
130-130: LGTM! But verify the authentication logic.The
authFilterfunction has been updated to check the root URL, aligning with the new endpoint structure.However, ensure that the authentication logic throughout the codebase is updated to match this change and does not introduce security issues.
Verification successful
Authentication logic verification complete.
The
authFilterfunction's logic is correctly updated to check the root URL, and there are no conflicting authentication mechanisms in the codebase.
authFilterfunction is only used innode/pkg/dal/utils/initializer/initializer.goand correctly checks the root URL.- Other authentication mechanisms found are unrelated to this specific change.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the authentication logic matches the updated endpoint structure. # Test: Search for authentication logic. Expect: Only occurrences of the new root URL. rg --type go -A 5 $'authFilter'Length of output: 876
Script:
#!/bin/bash # Description: Verify the authentication logic throughout the codebase to match the updated endpoint structure. # Search for other instances of authentication logic across the codebase. rg --type go -A 5 'auth|authentication|authFilter|validator'Length of output: 26486
node/pkg/dal/tests/api_test.go (4)
64-64: LGTM! Endpoint URL updated correctly.The endpoint URL has been updated to
http://localhost:8090/latest-data-feeds/allas per the new structure. This change is consistent with the PR objectives.
92-92: LGTM! Endpoint URLs updated correctly.The endpoint URLs have been updated to
http://localhost:8090/andhttp://localhost:8090/latest-data-feeds/test-aggregateas per the new structure. These changes are consistent with the PR objectives.Also applies to: 99-99
139-139: LGTM! Endpoint URL updated correctly.The endpoint URL has been updated to
http://localhost:8090/latest-data-feeds/test-aggregateas per the new structure. This change is consistent with the PR objectives.
166-166: LGTM! Endpoint URL updated correctly.The endpoint URL has been updated to
ws://localhost:8090/wsas per the new structure. This change is consistent with the PR objectives.
Description
simplify endpoint before official launch
AS IS
https://dal.baobab.orakl.network/api/v1/dal/symbolsTO BE
https://dal.baobab.orakl.network/symbolsthe healthcheck endpoints should also be updated from helmchart yaml files
Type of change
Please delete options that are not relevant.
Checklist before requesting a review
Deployment