feat: provide genes/transcripts endpoint with openapi (#605) #610
Conversation
|
Warning Rate limit exceeded@holtgrewe has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 53 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. WalkthroughThe changes introduce a new configuration file, 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 (
|
holtgrewe
force-pushed
the
605-provide-endpoint-alternative-with-openapi-schema-for-genestxs
branch
from
dd2f61a to
b612ae3
Compare
![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
🧹 Outside diff range and nitpick comments (6)
src/server/run/actix_server/mod.rs (1)
62-62: LGTM! Consider API versioning strategy.The addition of
handle_with_openapialongside the existinghandleendpoint is logically placed. However, having two endpoints for potentially the same functionality suggests a transition period.Consider:
- Documenting the deprecation timeline for the non-OpenAPI endpoint
- Adding OpenAPI support to all endpoints consistently
- Implementing API versioning through content negotiation or accept headers instead of parallel endpoints
src/server/run/actix_server/versions.rs (1)
44-63: Enhance error message in TryFrom implementation.The type conversion implementations are well-structured and handle all cases appropriately. However, the error message in the catch-all arm could be more descriptive.
Consider enhancing the error message to include the actual value:
- _ => Err(anyhow::anyhow!("Unsupported assembly")), + other => Err(anyhow::anyhow!("Unsupported assembly: {:?}", other)),src/server/run/actix_server/gene_txs.rs (4)
35-36: Use safer default conversion forgenome_buildThe expression
GenomeBuild::Grch37 as i32relies on the underlying representation of the enum, which can be prone to errors if the enum's definition changes. It's safer to use theinto()method for conversions.Apply this diff to improve the safety of the default value conversion:
- let genome_build = GenomeBuild::try_from(genome_build.unwrap_or(GenomeBuild::Grch37 as i32)) + let genome_build = GenomeBuild::try_from(genome_build.unwrap_or_else(|| GenomeBuild::Grch37.into()))
89-93: Align field optionality between query structsIn
GenesTranscriptsListQuery,hgnc_idandgenome_buildare required fields, whereas inGeneTranscriptsQuery, they are optional (Option<String>andOption<i32>respectively). This inconsistency may lead to confusion and potential errors during conversions.Consider making
hgnc_idandgenome_buildoptional inGenesTranscriptsListQueryto align withGeneTranscriptsQuery, or adjust the conversion logic to account for the differences in field optionality.
284-314: Simplify nested conversions inTranscriptimplementationThe nested
try_fromcalls within theTryFromimplementation forTranscriptcan make the code harder to read and maintain.Consider refactoring the nested conversions using helper functions or intermediary variables to enhance readability. For example:
let biotype = pbs::txs::TranscriptBiotype::try_from(value.biotype) .map_err(|e| anyhow::anyhow!("Invalid biotype: {}", e)) .and_then(TranscriptBiotype::try_from)?; let tags = value .tags .into_iter() .map(pbs::txs::TranscriptTag::try_from) .map(|res| res.and_then(TranscriptTag::try_from)) .collect::<Result<Vec<_>, _>>()?;
356-359: Provide more context in conversion error messagesThe error message
"Conversion error: {}"may not give sufficient context when a conversion fails inhandle_with_openapi.Enhance the error message to include more details, which can aid in debugging:
- CustomError::new(anyhow::anyhow!("Conversion error: {}", e)) + CustomError::new(anyhow::anyhow!("Failed to convert GeneTranscriptsResponse: {}", e))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
codecov.yml(1 hunks)src/server/run/actix_server/gene_txs.rs(4 hunks)src/server/run/actix_server/mod.rs(1 hunks)src/server/run/actix_server/versions.rs(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- codecov.yml
🔇 Additional comments (3)
src/server/run/actix_server/mod.rs (1)
Line range hint 62-67: Verify OpenAPI documentation generation.
The new endpoint should be properly documented in the OpenAPI schema that's being served at /api-docs/openapi.json.
✅ Verification successful
Let me gather more information about the OpenAPI documentation.
Let me check for the schema definition and API documentation structure.
Let me check the OpenAPI configuration in more detail.
Based on the gathered information, I can now provide the final response:
OpenAPI documentation is properly implemented for the endpoint
The endpoint is correctly documented with:
- Path operation decorator with
#[utoipa::path] - Proper response schema definition using
GenesTranscriptsListResponse - Integration with Swagger UI at
/swagger-uiand OpenAPI JSON at/api-docs/openapi.json - Complete response documentation including success (200) and error (500) cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new endpoint is documented in OpenAPI schema
# Test: Search for OpenAPI/Swagger documentation related to the genes/transcripts endpoint
rg -l "genes/transcripts" --type rust
Length of output: 97
Script:
#!/bin/bash
# Check for OpenAPI documentation in gene_txs.rs
rg "#\[utoipa" -A 5 src/server/run/actix_server/gene_txs.rs
# Check for any OpenAPI path definitions
ast-grep --pattern 'OpenApi::new().path($$$)'
Length of output: 388
Script:
#!/bin/bash
# Check for OpenAPI schema definitions in gene_txs.rs
rg "impl Schema" -A 5 src/server/run/actix_server/gene_txs.rs
# Check for the OpenAPI configuration
rg "ApiDoc" --type rust -A 5
Length of output: 1615
Script:
#!/bin/bash
# Check the OpenAPI configuration implementation
rg "impl ApiDoc" -A 10 src/server/run/mod.rs
# Check for schema components and responses
rg "GenesTranscriptsListResponse" --type rust
Length of output: 645
src/server/run/actix_server/versions.rs (2)
6-6: LGTM: Import addition is appropriate.
The new import for pbs is necessary for the type conversions and follows Rust's import conventions.
44-63: Verify usage of the new type conversions.
Let's ensure these conversions are being used consistently across the codebase.
✅ Verification successful
Based on the search results, I can now generate the final response:
Type conversions are used correctly and consistently
The new type conversions between Assembly and GenomeBuild in versions.rs are part of a well-structured type conversion chain in the codebase:
- Used in
gene_txs.rsfor handling genome build parameters in API requests - Integrates with existing conversions in
common/mod.rsbetweenGenomeRelease,Assembly, andGenomeBuild - Maintains consistency with the biocommons ecosystem through
biocommons_bioutils::assemblies::Assembly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of Assembly and GenomeBuild conversions
# Test: Find direct usage of the conversion methods
echo "=== Direct conversion usage ==="
rg "\.into\(\)" -A 2 -B 2 | grep -C 2 "Assembly.*GenomeBuild\|GenomeBuild.*Assembly"
# Test: Find potential places where conversion might be needed
echo "=== Potential conversion locations ==="
rg "GenomeBuild|Assembly" --type rust
Length of output: 10071
| @@ -87,7 +108,7 @@ impl DataVersionEntry { | |||
| } | |||
| } | |||
|
|
|||
| /// Response of the `/v1/version` endpoint. | |||
| /// Response of the `/api/v1/version` endpoint. | |||
There was a problem hiding this comment.
Fix documentation inconsistency.
The documentation comment mentions /api/v1/version but the actual endpoint path is /api/v1/versionsInfo.
Update the documentation to match the actual endpoint path:
-/// Response of the `/api/v1/version` endpoint.
+/// Response of the `/api/v1/versionsInfo` endpoint.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Response of the `/api/v1/version` endpoint. | |
| /// Response of the `/api/v1/versionsInfo` endpoint. |
| impl TryFrom<pbs::txs::GenomeAlignment> for GenomeAlignment { | ||
| type Error = anyhow::Error; | ||
|
|
||
| fn try_from(value: pbs::txs::GenomeAlignment) -> Result<Self, Self::Error> { | ||
| Ok(GenomeAlignment { | ||
| genome_build: Assembly::try_from(pbs::txs::GenomeBuild::try_from(value.genome_build)?)?, | ||
| contig: value.contig.clone(), | ||
| cds_start: value.cds_start, | ||
| cds_end: value.cds_end, | ||
| strand: Strand::from(pbs::txs::Strand::try_from(value.strand)?), | ||
| exons: value.exons.into_iter().map(Into::into).collect(), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle potential errors in GenomeAlignment conversion
In the TryFrom implementation for GenomeAlignment, the conversion of pbs::txs::GenomeAlignment to GenomeAlignment does not handle errors that may occur during the conversion of exons.
Update the exons mapping to handle possible errors:
- exons: value.exons.into_iter().map(Into::into).collect(),
+ exons: value
+ .exons
+ .into_iter()
+ .map(TryInto::try_into)
+ .collect::<Result<_, _>>()?,Also, adjust the function to return a Result<Self, Self::Error> that appropriately handles these potential conversion errors.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| impl TryFrom<pbs::txs::GenomeAlignment> for GenomeAlignment { | |
| type Error = anyhow::Error; | |
| fn try_from(value: pbs::txs::GenomeAlignment) -> Result<Self, Self::Error> { | |
| Ok(GenomeAlignment { | |
| genome_build: Assembly::try_from(pbs::txs::GenomeBuild::try_from(value.genome_build)?)?, | |
| contig: value.contig.clone(), | |
| cds_start: value.cds_start, | |
| cds_end: value.cds_end, | |
| strand: Strand::from(pbs::txs::Strand::try_from(value.strand)?), | |
| exons: value.exons.into_iter().map(Into::into).collect(), | |
| }) | |
| } | |
| } | |
| impl TryFrom<pbs::txs::GenomeAlignment> for GenomeAlignment { | |
| type Error = anyhow::Error; | |
| fn try_from(value: pbs::txs::GenomeAlignment) -> Result<Self, Self::Error> { | |
| Ok(GenomeAlignment { | |
| genome_build: Assembly::try_from(pbs::txs::GenomeBuild::try_from(value.genome_build)?)?, | |
| contig: value.contig.clone(), | |
| cds_start: value.cds_start, | |
| cds_end: value.cds_end, | |
| strand: Strand::from(pbs::txs::Strand::try_from(value.strand)?), | |
| exons: value | |
| .exons | |
| .into_iter() | |
| .map(TryInto::try_into) | |
| .collect::<Result<_, _>>()?, | |
| }) | |
| } | |
| } |
…alternative-with-openapi-schema-for-genestxs
holtgrewe
deleted the
605-provide-endpoint-alternative-with-openapi-schema-for-genestxs
branch
Summary by CodeRabbit
New Features
Assemblyenum and its interaction with genome builds.Bug Fixes
Documentation