Skip to content
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

fix response summary #311

Merged
merged 1 commit into from
Apr 11, 2024
Merged

Conversation

gsfk
Copy link
Contributor

@gsfk gsfk commented Apr 11, 2024

numTotalResults in the response summary is supposed to show the total number of results found by the query, regardless of pagination (see the spec).

In some cases, the reference implementation will spuriously return pagination values instead of the total number of results. Here is an example:

  • deploy beacon according to deploy/README.md
  • query /api/individuals with the following payload (taken from the readme), numTotalResults will be 31 using the example data provided.
{
    "meta": {
        "apiVersion": "2.0"
    },
    "query": {
        "requestParameters": {
    "alternateBases": "G" ,
    "referenceBases": "A" ,
"start": [ 16050074 ],
            "end": [ 16050568 ],
	    "variantType": "SNP"
        },
        "filters": [],
        "includeResultsetResponses": "HIT",
        "pagination": {
            "skip": 0,
            "limit": 10
        },
        "testMode": false,
        "requestedGranularity": "record"
    }
}
  • repeat the request, but change requestedGranularity to "count". numTotalResults will be 10 instead of 31, because build_response_summary() spuriously compares total results and pagination limit and returns whichever is smaller.

The fix is to not overwrite numTotalResults with a different value. There were a number of cases and one function parameter that are no longer necessary.

@gsfk gsfk mentioned this pull request Apr 11, 2024
@costero-e costero-e merged commit bd2fb14 into EGA-archive:master Apr 11, 2024
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants