-
Notifications
You must be signed in to change notification settings - Fork 12
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
Http api and http testing #84
base: unstable
Are you sure you want to change the base?
Conversation
merge test 1
branch check
tmp rm CROSS_FEATURES
use GH_TOKEN
hardcode docker repo, set dockerfile location
use correct dir
update rust and path
use Cargo.toml as source of truth
fix path for toml_reader.sh
debugging
add conditional imports for Unix target family in environment module
conditional imports for unix
Pr 56 review
|
||
/// OperatorMetadataDto model | ||
#[derive(Debug, Serialize, Deserialize, ToSchema)] | ||
pub struct OperatorMetadataDto { |
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.
I think the fields should stick with rust underscore convention and just use serde rename. Something like
#[serde(rename = "operatorName")] pub operator_name: Option<String>
use super::handlers::*; | ||
|
||
pub fn create_router() -> Router { | ||
Router::new() |
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.
might be nice to try to break these functions up into modules for a nice organizational hierarchy.
Something like a root handler module, than operator module, network module, cluster module, etc. Then instead of the long function name you could do handle::operator::get_operator
, handle::cluster::get_id
. Doesn't have to be that structure, just some separation.
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.
Thanks @Zacholme7 - yes, was thinking the same.
use utoipa::ToSchema; | ||
|
||
|
||
#[utoipa::path(GET, path = "/api/v4/{network}/accounts", tag = "Accounts", params(("network", String),("page", String),("perPage", String),), |
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.
these macros are quite verbose. im not very familiar with utopia so im not sure if this is the convention. is there any way to simplify these?
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.
The utoipa
macro seems to be the most common way to add openapi / swagger docs.
I get it though, it is verbose. But these openapi annotations do make it easier for api consumers.
These function signatures are generated from templates. So we can tweak the templates to our liking.
Nice start on this its good to get all these down and now we can start to nail down specifics! Left some early feedback with nitpick comments. |
Issue Addressed
This is related to #26 and related testing issues.
Proposed Changes
Proposal to use identical paths / endpoints and method names as the ssv openapi spec.
But, as these spec is incomplete in terms of response types, to improve it by being more rigorous with, in particular, response types.