HTTPError message does not shown when response return 504

Yamazaki-Youichi commented 5 years ago

Hello. Thank you for the awesome package. I have founded trivial things regarding the raise exception.

I would like to check which models are prepared. Could you provide me the list of trained models, for my research purpose?

In addition, I have a simple query to get all Models data. When server return the 504 status, the raise message did not match as your expectation.

mdl = MDL()
models = mdl("_", save_to=False)


C:\Anaconda3\lib\json\decoder.py in raw_decode(self, s, idx)
355             obj, end = self.scan_once(s, idx)
356         except StopIteration as err:
--> 357             raise JSONDecodeError("Expecting value", s, err.value) from None
358         return obj, end

JSONDecodeError: Expecting value: line 1 column 1 (char 0)

When 504 error, ```ret.json()``` doesn't success because the return value is not json, since
```JSONDecodeError``` overwrite ```raise HTTPError```.

```python
    def query(self, query, variables):
        payload = json.dumps({'query': query, 'variables': variables})
        ret = requests.post(url=self._url, headers=self._headers, data=payload)
        if ret.status_code != 200:
            raise HTTPError('status_code: %s, %s' %
                            (ret.status_code, ret.json()))
        ret = ret.json()['data']
        return ret

ipdb> ret
<Response [504]>
ipdb> ret.content
b'<html>\r\n<head><title>504 Gateway Time-out</title></head>\r\n<body bgcolor="white">\r\n<center><h1>504 Gateway Time-out</h1></center>\r\n<hr><center>nginx/1.14.2</center>\r\n</body>\r\n</html>\r\n'
ipdb> ret.json()
*** json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

I'm looking forward for your kind reply. Thank you very much.

TsumiNa commented 5 years ago

Hi @Yamazaki-Youichi, Thank you for your feedback!

If you want to list all these model informations. Use the method like this:

mdl = MDL()
results = mdl("", save_to=False)

The _ in python mean ignore some var but when use it a string that's not make sense.

for 1). Yes, as you said we need to publish our model list and also some additional informations as soon as possible. We wish that it can be done in next week.

for 2). I think the query string _ lead to this error because all modelsets' name contain character _. This lead to a very long time querying and a huge size response body. We are planning to add some simple query statements e.g listModeSets, listProperties and bring them to users in version 0.3.0.

TsumiNa commented 5 years ago

@Yamazaki-Youichi, I have created a new issue #65 to collect the suggestion of high priority API.

Yamazaki-Youichi commented 5 years ago

Thank you for your reply and created new issue! I will wait for the new version. I'm sorry to trouble you.

Regarding to the JSONDecodeError raise issue,
in my opinion, the raising of JSONDecodeError to overwrite HTTPError is misleading. For example, when server cause error by such bad query "_", it always needs to raise HTTPError. However, in the present code, since JSONDecodeError is raised, there is no indication of bad queries. I'm not good at coding but here is my suggestions:

    def query(self, query, variables):
        payload = json.dumps({'query': query, 'variables': variables})
        ret = requests.post(url=self._url, headers=self._headers, data=payload)
        if ret.status_code != 200:
            # Here, When 504 error, ret.json() will raise error.
            try:
                message = ret.json()
            except JSONDecodeError:
                message = "Server does not return value."
            raise HTTPError('status_code: %s, %s' %
                            (ret.status_code, message))
        ret = ret.json()['data']
        return ret

If it's not too much trouble, I will send pull request.

TsumiNa commented 5 years ago

@Yamazaki-Youichi Yes, we should raise an HTTPError to give clear information to users. Thanks for your pr!

Yamazaki-Youichi commented 5 years ago

Thank you! I sent PR at https://github.com/yoshida-lab/XenonPy/pull/66

Yamazaki-Youichi commented 5 years ago

Close

yoshida-lab / XenonPy

HTTPError message does not shown when response return 504 #64