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

consistent response format #79

Closed
francoisromain opened this issue Nov 22, 2017 · 10 comments
Closed

consistent response format #79

francoisromain opened this issue Nov 22, 2017 · 10 comments

Comments

@francoisromain
Copy link

francoisromain commented Nov 22, 2017

Sometimes the api response is a message (ie: login respond with "success": "Authentication succeeded.") sometimes it's an object (ie: me respond with { "_id": "555299eff80f910100d741d1", "description": "", "role": "user", "username": "johndoe" }). It would be cleaner to make it consistent.

For example follow jsend specs:

Successful request:

{
  "status": "success",
  "data": {
    /* Application-specific data would go here. */
  },
  "message": null /* Or optional success message */
}

Failed request:

{
  "status": "error",
  "data": null, /* or optional error payload */
  "message": "Error xyz has occurred"
}
@francoisromain
Copy link
Author

following up: login could return infos about the logged-in user:

{
  "status": "success",
  "data": {
    "_id": 123456,
    "name": "Lulu",
    "description": "Lulu desc…"
  },
  "message": "User logged in"
}

hbredin added a commit that referenced this issue Dec 3, 2017
Addresses #79

This is a breaking change in the output of the API.
Existing clients will likely need to be updated.

Note that "metadata" and "server sent events" still need to be updated
@hbredin
Copy link
Member

hbredin commented Dec 3, 2017

I just started a new branch which should address the main point: consistent response format.

hbredin added a commit that referenced this issue Dec 6, 2017
fixes #79 except for /metadata/ and /watch/ routes
@hbredin
Copy link
Member

hbredin commented Dec 6, 2017

Available for testing in branch develop since commit 15bee85

Note that /login route has not yet been updated but a new issue #83 has been created to keep track of this request.

@hbredin hbredin closed this as completed Dec 6, 2017
@francoisromain
Copy link
Author

francoisromain commented Dec 8, 2017

It seems to work fine, but now there is an error 500 on update actions with corpora, media, layers and annotations (updates only work fine on users and groups).

@hbredin
Copy link
Member

hbredin commented Dec 8, 2017

I cannot reproduce the error locally:

>>> from camomile import Camomile
>>> client = Camomile('http://localhost:3000')
>>> client.login(USERNAME, PASSWORD)
>>> result = client.updateCorpus(CORPUS_ID, description={'test': 'test'})
>>> print(result['status'])
success
>>> print(result['data']['description'])
{'test': 'test'}

Can you please clarify?

@hbredin hbredin reopened this Dec 8, 2017
@francoisromain
Copy link
Author

francoisromain commented Dec 8, 2017

When I try to update a corpus / media / layer / annotation from camomile-ui, using camomile-server and camomile-client-javascript, it returns an error 500.

The error logged from camomile-ui does not say much:

Warning: a promise was rejected with a non-error: [object Undefined]

It comes from the bluebird library used by request-promise (http://bluebirdjs.com/docs/warning-explanations.html)

The error logged from camomile-client-javascript points to MongoDB:

"StatusCodeError: 500 - {"status":"error","data":null,"message":{"name":"MongoError","message":"Unknown modifier: $pushAll","driver":true,"index":0,"code":9,"errmsg":"Unknown modifier: $pushAll"}}
    at new StatusCodeError (webpack-internal:///../camomile-client-javascript/node_modules/request-promise-core/lib/errors.js:32:15)

$pushAll is deprecated since v.2.4 of mongodb (-> https://docs.mongodb.com/manual/reference/operator/update/push/)

Do you have an idea?

@hbredin
Copy link
Member

hbredin commented Dec 8, 2017

Might be related to Automattic/mongoose#5870

@hbredin
Copy link
Member

hbredin commented Dec 8, 2017

and this: Automattic/mongoose#4455

@hbredin
Copy link
Member

hbredin commented Dec 8, 2017

Downgrading MongoDB to < 3.6 should solve the problem until mongoose 5.x is out.

hbredin added a commit that referenced this issue Dec 8, 2017
@hbredin
Copy link
Member

hbredin commented Dec 8, 2017

Just opened issue #85 as this is in fact not related to this one...
Closing this one, then...

@hbredin hbredin closed this as completed Dec 8, 2017
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

2 participants