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

Paging in unordered collections #58

Closed
3 tasks
rosibaj opened this issue Jan 8, 2021 · 4 comments
Closed
3 tasks

Paging in unordered collections #58

rosibaj opened this issue Jan 8, 2021 · 4 comments
Assignees
Labels
bug Something isn't working critical This MUST be completed for release Scope Change SP:3 Issue is estimated at 3 story points

Comments

@rosibaj
Copy link

rosibaj commented Jan 8, 2021

From Investigation in icgc-argo/donor-submission-aggregator#122, repeated queries to the rdpc-api is showing that the order of the response from rdpc is not guaranteed.

So when we pull 3 pages, there could be duplicated or missing analyses

It should be default sorted by the object-id (analysisid, run, id etc)

Song-search needs something like this:
https://github.com/icgc-argo/song-search/blob/develop/src/main/java/bio/overture/songsearch/repository/AnalysisRepository.java#L136-L147

  • Include additional paging information from the graphql api:

  • have the total number of hits, total returned in this page, and an indicator if there are more pages

  • currently, we get no page data back in the responses, so if we made our size param greater than your max size we would have to guess if there are more pages or if you just limited the search size

  • Please note if this is a breaking-change for workflow-ui. We need to update there with this ticket if so.

@rosibaj rosibaj added the major High Priority label Jan 8, 2021
@joneubank
Copy link
Member

joneubank commented Jan 8, 2021

Adding the query we were working with:

fragment AnalysisData on Analysis {
    analysisId
    analysisType
    donors {
      donorId
    }
  }
  query($analysisFilter: AnalysisFilter, $analysisPage: Page, $workflowRepoUrl: String) {
    analyses(
      filter: $analysisFilter,
      page: $analysisPage
      
    ) {
      ...AnalysisData
      runs: inputForRuns(
        filter: {
          repository: $workflowRepoUrl
        }
      ) {
        
        runId
        state
        repository
        inputAnalyses {
          analysisId
        }
      }
    }
  }

Using this for query vars:

{ "analysisFilter": 
  {
        "analysisState": "PUBLISHED",
        "analysisType": "sequencing_alignment",
        "studyId": "PACA-CA"
	},
  "analysisPage": {
    "from": 0,
    "size": 100
  },
"workflowRepoUrl": "https://github.com/icgc-argo/sanger-variant-calling.git"
}

@jaserud
Copy link
Contributor

jaserud commented Jan 18, 2021

RDPC-gateway changes to queries for analyses, runs, tasks and files :

  • top level queries are now inside a new object called content.
  • sorts is a new param for the queries with query specific fieldNames and option to order asc or desc:
  • info is a new object returened which contains totalHits, hasNextFrom, contentCount

See example illustrating new changes with analyses query
Before:

query {
  analyses  (
    filter: {analysisType:"sequencing_experiment"}
    page: {from:0, size:10}
 ) { 
      analysisId
      analysisType
  }
}

Now (sorts is a new query param wasn't there before):

query {
  analyses (
    filter: {analysisType:"sequencing_experiment"}
    page: {from:0, size:10}, 
    sorts:{fieldName:analysisId, order: asc}
  ) {
    content {
      analysisId
      analysisType
    }
    info {
      totalHits
      hasNextFrom
      contentCount
    }
  }
}

@jaserud
Copy link
Contributor

jaserud commented Jan 18, 2021

Notes about services that are connected to this feature:

Service Version Chart Misc Notes
song-search 2.5.0 no chart changes requries rdpc-gateway restart
workflow-search 2.6.0 no chart changes requries rdpc-gateway restart
workflow-ui 3.3.0 no chart changes (for this feature) updating above two will break ui for versions before this

@rosibaj
Copy link
Author

rosibaj commented Jan 22, 2021

Confirmed on release Jan 22; new query format is available at https://api.rdpc.cancercollaboratory.org/graphql

@rosibaj rosibaj closed this as completed Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical This MUST be completed for release Scope Change SP:3 Issue is estimated at 3 story points
Projects
None yet
Development

No branches or pull requests

3 participants