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

Handle failed requests for available controlled terms gracefully #54

Closed
5 of 6 tasks
alyssadai opened this issue Jan 11, 2024 · 1 comment · Fixed by #65
Closed
5 of 6 tasks

Handle failed requests for available controlled terms gracefully #54

alyssadai opened this issue Jan 11, 2024 · 1 comment · Fixed by #65
Assignees
Labels
released This issue/pull request has been released.

Comments

@alyssadai
Copy link
Contributor

alyssadai commented Jan 11, 2024

Questions:

  • should the response format be the same as for partial-success query results, e.g. with a section for the errors encountered?
  • should "partial-success" terms fetching be allowed?

Decision:
If query tool fails to get terms from a node:

  • For now, just return terms available from nodes that are reachable/don't error out
  • Should keep the same response shape as for cohort queries e.g.:
{
  "nb:Assessment": [
    {
      "TermURL": "cogatlas:trm_56a9137d9dce1",
      "Label": "behavioral approach/inhibition systems"
    },
    {
      "TermURL": "cogatlas:trm_55a6a8e81b7f4",
      "Label": "Barratt Impulsiveness Scale"
    },
    ...
}

to:

{
  "errors": [{}] ...
  "responses":
  {
    "nb:Assessment": [
      {
        "TermURL": "cogatlas:trm_56a9137d9dce1",
        "Label": "behavioral approach/inhibition systems"
      },
      {
        "TermURL": "cogatlas:trm_55a6a8e81b7f4",
        "Label": "Barratt Impulsiveness Scale"
      },
      ...
  },
  "nodes_response_status": "..."
}

When request to all nodes fail:

{
  "errors": [<all errors>] ...
  "responses":
  {
    "nb:Assessment": []
  },
  "nodes_response_status": "fail"
}

Steps to implement:

  • Switch to async requests to nodes
  • Add error responses to list
  • Add new response model for combined attribute response
  • Rename tests of partial success cohort queries to be more specific
  • Consider refactoring mixed-status JSONResponse into its own class/function?
  • When requests to all nodes fail, should return "responses": {<data_element_URI>: []}
@alyssadai alyssadai added flag:discuss Flag issue that needs to be discussed before it can be implemented. flag:schedule Flag issue that should go on the roadmap or backlog. labels Jan 11, 2024
@alyssadai alyssadai removed the flag:discuss Flag issue that needs to be discussed before it can be implemented. label Jan 11, 2024
@alyssadai alyssadai moved this to Backlog in Neurobagel Jan 11, 2024
@alyssadai alyssadai added bug:functional and removed flag:schedule Flag issue that should go on the roadmap or backlog. labels Jan 11, 2024
@alyssadai alyssadai moved this from Backlog to Specify - Active in Neurobagel Jan 29, 2024
@alyssadai alyssadai moved this from Specify - Active to Specify - Done in Neurobagel Jan 29, 2024
@alyssadai alyssadai moved this from Specify - Done to Implement - Active in Neurobagel Jan 29, 2024
@alyssadai alyssadai self-assigned this Jan 29, 2024
@alyssadai alyssadai moved this from Implement - Active to Implement - Done in Neurobagel Jan 31, 2024
@surchs surchs moved this from Implement - Done to Review - Active in Neurobagel Jan 31, 2024
@github-project-automation github-project-automation bot moved this from Review - Active to Review - Done in Neurobagel Feb 6, 2024
@surchs
Copy link
Contributor

surchs commented Apr 11, 2024

🚀 Issue was released in v0.1.0 🚀

@surchs surchs added the released This issue/pull request has been released. label Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants