-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: endpoint /api/v1/genes/lookup with OpenAPI (#587) #589
feat: endpoint /api/v1/genes/lookup with OpenAPI (#587) #589
Conversation
Caution Review failedThe pull request is closed. WalkthroughThe changes in this pull request focus on updating the gene lookup functionality in the server's API. The endpoint has been changed from Changes
Possibly related issues
Possibly related PRs
Poem
Warning Rate limit exceeded@holtgrewe has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 19 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. 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 (
|
66055a2
to
19bf233
Compare
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: 0
🧹 Outside diff range and nitpick comments (4)
src/server/run/genes_lookup.rs (4)
65-66
: Update the function docstring to reflect accurate functionalityThe docstring for the
handle
function currently states:/// Query for annotations for one variant.
However, this function handles gene lookups, not variant annotations. Please update the docstring to accurately describe its purpose to maintain clarity and consistency in the codebase.
89-89
: Complete the incomplete docstringThe docstring for
GenesLookupResponse
is incomplete:/// Result for `async fn handle_with_openapi(
Please provide a full description of the
GenesLookupResponse
struct to enhance code readability and maintain proper documentation standards.
81-81
: Remove the unnecessary trailing comma in the commentThe comment for the
query
field has an extra comma:/// The query string,
Please remove the trailing comma for grammatical correctness:
/// The query string
68-76
: Deprecation notice for the old endpointThe function
handle
still provides the deprecated/genes/lookup
endpoint. To assist users in migrating to the new API version, consider adding a deprecation warning or response header indicating that this endpoint will be removed in future releases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/server/run/genes_lookup.rs
(4 hunks)src/server/run/mod.rs
(4 hunks)
🔇 Additional comments (4)
src/server/run/mod.rs (4)
41-41
: LGTM: Clean import of new gene lookup types
The import follows the module's naming conventions and is properly grouped with other response type imports.
58-58
: LGTM: Proper integration of gene lookup handler in OpenAPI paths
The handler is correctly placed and maintains the alphabetical ordering of endpoints.
120-121
: LGTM: Complete schema documentation for gene lookup types
Both response and entry types are properly included in the OpenAPI components schemas, maintaining alphabetical order.
149-149
: Verify the endpoint transition strategy
The code shows both genes_lookup::handle
and genes_lookup::handle_with_openapi
services being registered. While this allows for a smooth transition, we should ensure there's a plan to:
- Mark the old endpoint as deprecated
- Communicate the deprecation to API consumers
- Eventually remove the old endpoint
✅ Verification successful
Transition strategy is already properly implemented
The code shows a well-implemented transition strategy:
- The old endpoint
/genes/lookup
is properly marked as deprecated in the module documentation. - The new endpoint
/api/v1/genes/lookup
is documented with OpenAPI. - Both endpoints share the same implementation through
handle_impl
, ensuring consistent behavior. - The transition is part of a broader API versioning effort (v1) as seen in the changelog.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the old endpoint is marked as deprecated in the handler
rg -A 5 "genes_lookup::handle.*deprecated"
# Look for any documentation about the deprecation
fd README.md --exec grep -A 5 "genes/lookup.*deprecated" {} \;
Length of output: 112
Script:
#!/bin/bash
# Check both handler implementations
ast-grep --pattern 'fn handle($$$) {
$$$
}'
# Look for any OpenAPI documentation or deprecation notices
rg -l "genes.*lookup"
# Check if there are any deprecation markers in the code
rg -i "deprecat"
Length of output: 533
Script:
#!/bin/bash
# Check the specific content of genes_lookup.rs to see how deprecation is handled
cat src/server/run/genes_lookup.rs
# Check changelog for any mentions of the transition
cat CHANGELOG.md
Length of output: 41745
Summary by CodeRabbit
New Features
/api/v1/genes/lookup
, with the previous endpoint marked as deprecated.GenesLookupQuery
and response structuresGenesLookupResponse
andGenesLookupResultEntry
for improved clarity and functionality.Bug Fixes
Documentation