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

The response data type of the management interface is not uniform #111

Open
pengjiejason opened this issue Dec 22, 2021 · 6 comments
Open

Comments

@pengjiejason
Copy link

I found that the responset type in mesh is divided into about 4 kinds.
I think the first one is normal, so I have tried to list as many interfaces as possible for the last three.

  1. Headers: Content-Type: application/json. The return data is also json. Most api's return this way.
  2. Headers: Content-Type: text/plain; charset=utf-8. The return data is json.
    • /apis/v1/mesh/traffictargets
    • /apis/v1/mesh/customresources
    • /apis/v1/mesh/httproutegroups
  3. Headers: Content-Type: text/vnd.yaml. The return data is yaml.
    • /apis/v1/objects/easemesh-controller
  4. Headers: Content-Type: text/plain; charset=utf-8. The return data is yaml.
    • apis response status is 40X. eg: /apis/v1/mesh/traffictargets/nameNotExist
@localvar
Copy link
Contributor

Yes, the 1st one is the correct way.

The 2nd: it is a bug, we have set the 'Content-Type' to 'application/json' in the code, but it is not working. I will fix it.
The 3rd: we have to keep it as it is, the mesh APIs return JSON and the Easegress APIs return YAML by default, and this is an Easegress one.
The 4th: as the status code is not 2XX and the body is empty, all APIs haven't set the content type, can we just ignore it?

@zhao-kun
Copy link
Contributor

zhao-kun commented Dec 22, 2021

About 4th, I agree @pengjiejason , we need to uniform content-type settings.

@zhao-kun
Copy link
Contributor

I think we need to clarify our API contract:

  • API for the EaseMesh should set Content-Type: application/json no matter the status code is 2XX or others.
  • API for the Easegress should set Content-Type: text/vnd.yaml no matter the status code is 2XX or others.
  • Path start with /apis/v1/mesh is an API for the EaseMesh.
  • Path start with /apis/v1/objects is an API for the Easegress.

@localvar
Copy link
Contributor

For non-2XX responses, theoretically, it is possible that the request hasn't been processed by our code, so I think the client should not be so strict on them.

@zhao-kun
Copy link
Contributor

Yes, the request hasn't been processed the Content-type can be undefined, but if the request has been processed, sever should return Content-Type in a consistent way (it's doesn't matter with the client, it's the server's behavior, the server's behavior should be consistency).

@localvar
Copy link
Contributor

the 2nd issue has been fixed in PR easegress-io/easegress#430.
the 4th is a little complex and needs more time to fix.

xxx7xxxx pushed a commit to easegress-io/easegress that referenced this issue Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants