search

normativeai / backend

GNU General Public License v3.0
3 stars 3 forks source link

Change mleancop timeout messages to give more information #27

Closed shaolintl closed 5 years ago

shaolintl commented 5 years ago

Right now, it reports a server error. We should assume that timeout happens mainly on failing to prove and report a more informative messages as follows:

@lex-lex Right now we have green, red and blue colors. Should we have a yellow one for these kinds of message? Which JSON object should I return?

lex-lex commented 5 years ago

The information alerts use the bootstrap style names for coloring, e.g. "warning" is yellow, "danger" is red, "success" is green, "info" is blue. If you want to decide the color of the response from the back end side, I'd suggest you include this information in the response, e.g. using an additional attribute type in the JSON.

What do you think?

shaolintl commented 5 years ago

@lex-lex Good idea. I have implemented it for now only for consistency and independence of theories and norms. You will get error code 206 and json {message, type}. Once you implement it and it works, I will do it also for queries.

shaolintl commented 5 years ago

@lex-lex I reverted the change as right now it breaks consistency and independence checks. I have moved the change to branch timeout so you can test it.

lex-lex commented 5 years ago

@shaolintl Could you please merge it into master s.t. I can test it without switching back and forth between different back end version? Just don't update nai.uni.lu as long as the front end is not compatible.

shaolintl commented 5 years ago

@lex-lex The update from master is automatic. Should I create a branch development which will contain the latest code and against which you will keep working?

lex-lex commented 5 years ago

Sounds reasonable, thanks!

shaolintl commented 5 years ago

@lex-lex Ok, the branch is called dev and will always contain the latest pushed code.

lex-lex commented 5 years ago

Ok, messages are included. You can merge that feature into master now.

shaolintl commented 5 years ago

In fact, I did that already by mistake..

On Sat, 15 Jun 2019 at 00:03, Alexander Steen notifications@github.com wrote:

Ok, messages are included. You can merge that feature into master now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/normativeai/backend/issues/27?email_source=notifications&email_token=ABMPYVL42L4LUZZG5HP7OEDP2QIRXA5CNFSM4HSCOCU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXYGMBI#issuecomment-502294021, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMPYVMIKO5ZMJX2YULZUQ3P2QIRXANCNFSM4HSCOCUQ .

lex-lex commented 5 years ago

Also, I locally merged the master before into dev because the vocabulary was not included in dev. just fyi :)

lex-lex commented 5 years ago

So this is resolved?

lex-lex commented 5 years ago

It seems that the answers of the back end are not consistently using the proposed format. When doing e.g. a successful independence check, it will just return {data: {independent: true}} which will not be interpreted correctly as it does not contain message and type. Can you update this?

shaolintl commented 5 years ago

Fixed, sorry. I misunderstood but it makes lots of sense.