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

phase 1: changes to async-job-checking and asyncquery endpoints #602

Closed
colleenXu opened this issue Mar 29, 2023 · 7 comments
Closed

phase 1: changes to async-job-checking and asyncquery endpoints #602

colleenXu opened this issue Mar 29, 2023 · 7 comments
Assignees

Comments

@colleenXu
Copy link
Collaborator

colleenXu commented Mar 29, 2023

Basic summary:

  • The TRAPI PR describes /async_query_status/{job_id} endpoint and response format AsyncQueryStatusResponse
    • we'll probably update the existing check_query_status endpoint...another option is to make a new endpoint?
    • not sure if we're planning to "stream" logs / progress / queue-position using this endpoint...
  • This PR ALSO describes an update to /asyncquery response format...they now require the job_id (look up AsyncQueryResponse in the linked PR)
  • Note that updating the SmartAPI yamls (another issue) will be needed to advertise the /async_query_status/{job_id} endpoint since it's new

I'll leave it to others like @tokebe or @newgene to add details/requirements as needed.

From bullet 5 of changelog:

Add /async_query_status/{job_id} endpoint https://github.com/NCATSTranslator/ReasonerAPI/pull/395/files

  • timing: phase 1. Jackson says this is easy + quick to knock out.
  • priority: medium? get it done and move on?
  • responsible dev: Jackson
@colleenXu colleenXu changed the title trapi 1.4: update existing async-job-checking endpoint to match new standard? update existing async-job-checking endpoint to match new standard? Mar 29, 2023
@colleenXu colleenXu changed the title update existing async-job-checking endpoint to match new standard? update existing async-job-checking endpoint to match new standard Mar 29, 2023
@colleenXu colleenXu changed the title update existing async-job-checking endpoint to match new standard update async endpoint response + async-job-checking endpoint Mar 29, 2023
@colleenXu colleenXu changed the title update async endpoint response + async-job-checking endpoint changes to async-job-checking and asyncquery endpoints Mar 29, 2023
@colleenXu
Copy link
Collaborator Author

colleenXu commented Mar 30, 2023

Noting some info from @tokebe on Slack, regarding async-job's response "status" field:

The current complete set of possibilities are completed, failed, delayed, active, waiting, paused, stuck or null because statuses come from Bull, however after TRAPI 1.4 we'll have to change these to one of Queued, Running, Completed, Failed

Just a note, you shouldn't ever see delayed, paused, stuck, or null with the current code

@colleenXu
Copy link
Collaborator Author

colleenXu commented Apr 4, 2023

Update notes: opening post was last updated 4/4 evening

  • TRAPI 1.4 may still change. When those changes happen, they should be on this branch and documented in this branch's changelog

@colleenXu colleenXu changed the title changes to async-job-checking and asyncquery endpoints phase 1: changes to async-job-checking and asyncquery endpoints Apr 5, 2023
@colleenXu
Copy link
Collaborator Author

@tokebe's question and its answer:

Question:

RE: TRAPI 1.4 Asyncquery status.
Given the spec of AsyncQueryResponse, it's implied that we could return a non-Accepted status, such as in cases of an invalid query. However, job_id is non-nullable. In the event of an invalid query, for instance, something that would fit QueryNotTraversable, is it acceptable to return that status and a description, but not an ID? Or should a junk ID be returned, such as job_id: REJECTED?

Answer to implement: we intended that a bad request would return a 400 or other HTTP code, at which point AsyncQueryResponse and job_id are not required....

References:

@tokebe
Copy link
Member

tokebe commented Apr 12, 2023

No new code required to address the current answer to my question, unless something changes, the only thing left is updating our own SmartAPI.

@colleenXu
Copy link
Collaborator Author

colleenXu commented Apr 12, 2023

Note that biothings.io/explorer runs async queries in its "try it" page by using the ITRB prod instance (this issue).

If we push these updates up to ITRB prod, we may need to check that this webpage still works..

@colleenXu
Copy link
Collaborator Author

colleenXu commented Jul 19, 2023

Going to close this because all instances are deployed with the code related to this issue.

EDIT: I'd like @tokebe to confirm that the requirements of this issue have been fulfilled and if so, close this issue...

@colleenXu colleenXu reopened this Jul 19, 2023
@tokebe
Copy link
Member

tokebe commented Jul 20, 2023

Requirements fulfilled, related follow-up work can be tracked under #671

@tokebe tokebe closed this as completed Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants