-
Notifications
You must be signed in to change notification settings - Fork 32
🧑💻 Iterate over detectors API to include multiple endpoints #40
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
🧑💻 Iterate over detectors API to include multiple endpoints #40
Conversation
Signed-off-by: gkumbhat <[email protected]>
docs/api/openapi_detector_api.yaml
Outdated
- type | ||
- loc | ||
- msg | ||
- type |
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.
If we're expecting detectors to give these validation errors I think we should revisit this, since this "loc", "type" concept is based on FastAPI and may not make sense generally for most detector servers
docs/api/openapi_detector_api.yaml
Outdated
role: | ||
allOf: | ||
- $ref: '#/components/schemas/RoleEnum' | ||
description: 'Who wrote the message: [<enum ''RoleEnum''>]' |
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.
Based on this description I would think something like author
may be more appropriate? "Role" of a message seems a bit open-ended, like "what is the role of this message?" e.g. persuasion. Or perhaps the description may need to be more specific?
Signed-off-by: gkumbhat <[email protected]>
application/json: | ||
schema: | ||
$ref: '#/components/schemas/Error' | ||
/api/v1/text/context/doc: |
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.
Could we potentially add in the summary and/or description how doc
plays into this endpoint?
…tion example Signed-off-by: gkumbhat <[email protected]>
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.
LGTM
Changes
Note: API is still in iteration and we will update the orchestrator client once its finalized / stable in story #37