-
Notifications
You must be signed in to change notification settings - Fork 5
Implemented the service-info endpoint #43
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
Conversation
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.
Looks good for now.
The service-info is still being discussed in ga4gh/refget#39 so I'm sure their will be other changes
"type": "string" | ||
} | ||
}, | ||
"sorted_names_length_pairs":{ |
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 made a typo in the issue and we settled on using hyphen instead of underscore.
Of course we should make sure the names match between the service info and the API but the correct one is the one bellow.
"sorted_names_length_pairs":{ | |
"sorted-name-length-pairs":{ |
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.
actually we decided on underscores, right?
"documentationUrl": "https://www.ebi.ac.uk/eva/....", | ||
"updatedAt": "08-24-2023", | ||
"environment": "dev", | ||
"version": "tagged version", |
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.
As a future improvement maybe version
and updatedAt
should be filled dynamically, even if the rest isn't.
Also @tcezard, what's the difference between this version
and type.version
? This one is the code version and the other is the spec version?
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.
For the versions:
- version is the version of the service
- type.version is the version of the API
}, | ||
"sorted_names_length_pairs":{ | ||
"type": "array", | ||
"description": "objects composed of length and name of a sequence digested and ordered lexicographically", |
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.
"description": "objects composed of length and name of a sequence digested and ordered lexicographically", | |
"description": "Objects composed of length and name of a sequence digested and ordered lexicographically.", |
It should be |
@nsheff I'll raise an issue for that to do the necessary refactoring |
closes #37
Implemented the service-info endpoint.
I actually used the same file provided here by @tcezard.
I did one change in the
lengths
attribute's type that I changed it intostring
, which is how it is defined in our API, and we'll consider updating it intointeger
after we do the refactor in our API.I would like to know whether we should use
sorted_names_length_pairs
, as provided in the service-info file, orsorted_name_length_pairs
as we defined it in our java model.