-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Update history api to array #8890
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
base: master
Are you sure you want to change the base?
Changes from all commits
80ea3d8
4a21727
565b537
a2b8688
fdc2a53
36e7363
0609136
c9a5ecd
e79f925
b8963ef
72ec6ce
b64df11
94684db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -652,6 +652,22 @@ async def get_history_prompt_id(request): | |
prompt_id = request.match_info.get("prompt_id", None) | ||
return web.json_response(self.prompt_queue.get_history(prompt_id=prompt_id)) | ||
|
||
@routes.get("/history_v2") | ||
async def get_ordered_history(request): | ||
max_items = request.rel_url.query.get("max_items", None) | ||
if max_items is not None: | ||
max_items = int(max_items) | ||
|
||
offset = request.rel_url.query.get("offset", 0) | ||
offset = int(offset) | ||
|
||
return web.json_response(self.prompt_queue.get_ordered_history(max_items=max_items, offset=offset)) | ||
|
||
@routes.get("/history_v2/{prompt_id}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we agreed we'd update this endpoint as well cc: @guill @webfiltered There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this was specifically my recommendation to avoid a case where people need to remember to use one endpoint for the list and one for individual items (which goes against normal REST principles). It's much easier if we're just able to say "move to this new endpoint as we'll be deprecating I don't have a strong opinion on the specific name we use, but do strongly feel we should keep this route to preserve consistency. |
||
async def get_history_v2_prompt_id(request): | ||
prompt_id = request.match_info.get("prompt_id", None) | ||
return web.json_response(self.prompt_queue.get_history_v2(prompt_id=prompt_id)) | ||
|
||
@routes.get("/queue") | ||
async def get_queue(request): | ||
queue_info = {} | ||
|
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.
use a better name like ordered_history
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.
I think the decision was to use this name because we're moving from the old endpoint later on cc: @webfiltered @guill to confirm
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.
Yeah, the intention was to deprecate the
/history
endpoint eventually in favor of this one (so we don't have to maintain both).I don't have a particularly strong opinion on the name though. Any of:
history_v2
history_info
workflow_history
user_history
prompt_history
ordered_history
seem reasonable to me.