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

wrong response when delete conversation by api #5401

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

chivalry1314
Copy link

@chivalry1314 chivalry1314 commented Jun 19, 2024

Description

wrong response when delete conversation by api

Fixes #5400

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

image
image
image

  • TODO

Suggested Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods
  • optional I have made corresponding changes to the documentation
  • optional I have added tests that prove my fix is effective or that my feature works
  • optional New and existing unit tests pass locally with my changes

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. 🐞 bug Something isn't working labels Jun 19, 2024
crazywoola
crazywoola previously approved these changes Jun 19, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 19, 2024
@takatost takatost requested a review from crazywoola June 19, 2024 12:46
@takatost
Copy link
Collaborator

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/DELETE
Returning a 204 status code is reasonable and compliant with the standards. You can refer to this specification link for more details.

@takatost takatost requested review from takatost and removed request for crazywoola June 19, 2024 12:47
@chivalry1314
Copy link
Author

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/DELETE Returning a 204 status code is reasonable and compliant with the standards. You can refer to this specification link for more details.

but if I use 204 status code I cannot get the success message as the dify document says from the server like the first picture I offered. the status code 204 means no further information is to be supplied.

@takatost
Copy link
Collaborator

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/DELETE Returning a 204 status code is reasonable and compliant with the standards. You can refer to this specification link for more details.

but if I use 204 status code I cannot get the success message as the dify document says from the server like the first picture I offered. the status code 204 means no further information is to be supplied.

This is correct, so the direction for the fix could be to remove the response content from the API implementation and documentation, while keeping the 204 status code. Thank you.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jun 20, 2024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remain this file untouched in main branch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not related as well.

@dosubot dosubot bot removed the lgtm This PR has been approved by a maintainer label Jun 20, 2024
@takatost
Copy link
Collaborator

takatost commented Jul 4, 2024

https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/DELETE Returning a 204 status code is reasonable and compliant with the standards. You can refer to this specification link for more details.

but if I use 204 status code I cannot get the success message as the dify document says from the server like the first picture I offered. the status code 204 means no further information is to be supplied.

204 status code of means the execution was successful, so you can ignore the success message internal.

@@ -69,6 +70,13 @@ def pagination_by_first_id(cls, app_model: App, user: Optional[Union[Account, En

history_messages = list(reversed(history_messages))

# 进行特殊处理,截取“问询”字段
for hismessage in history_messages:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this particular piece of logic

@@ -58,7 +58,9 @@ class MessageListApi(Resource):
'conversation_id': fields.String,
'inputs': fields.Raw,
'query': fields.String,
'message_tokens': fields.Integer,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here are not in line with the topic of current PR, please create a new PR separately, thanks!

- "deploy/dev"
release:
types: [published]
branches: [ main ]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please restore this workflow to its original state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong response when delete conversation by api
3 participants