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

uncaught issue in restapi #1390

Closed
ltalirz opened this issue Apr 9, 2018 · 8 comments
Closed

uncaught issue in restapi #1390

ltalirz opened this issue Apr 9, 2018 · 8 comments

Comments

@ltalirz
Copy link
Member

ltalirz commented Apr 9, 2018

The current schema does not contain an entry for cifdata.

A request like http://localhost:5000/api/v2/cifs/ therefore returns an internal server error.

The interesting thing is that there is a test for the cifs endpoints and it passes:
https://github.com/aiidateam/aiida_core/blob/7b3113c7a9b998a8685e49e31a1cac8d2d2a997d/aiida/backends/tests/restapi.py#L792-L805

The reason seems to be that it is using the app.test_client() (?).
@waychal We need to find a way of testing that the schema is complete.
Anyhow, it probably makes sense to wait until the schema has been integrated in the source code in a "proper" way.

@waychal
Copy link
Contributor

waychal commented Apr 9, 2018

In above test case schema is not required to download cif data. So if there is problem to get node schema, above test should and will pass.

We need to add separate testcases to check schema

@ltalirz
Copy link
Member Author

ltalirz commented Apr 12, 2018

In above test case schema is not required to download cif data.

@waychal What I don't understand is why this line
https://github.com/aiidateam/aiida_core/blob/7b3113c7a9b998a8685e49e31a1cac8d2d2a997d/aiida/backends/tests/restapi.py#L799
is different from typing http://localhost:5000/api/v2/cifs/7b3113c7a9b998a8685e49e31a1cac8d2d2a997d/content/download in the browser (which causes the internal server error).

If you let me know, I can prepare the test.
We should fix this soon, since all the /cifs/ endpoints are broken now.

@ltalirz ltalirz added this to the v0.12.0 milestone Apr 26, 2018
@ltalirz
Copy link
Member Author

ltalirz commented Apr 26, 2018

@waychal Are you going to work on this soon?
I can easily fix the missing entry in the schema, but as I mentioned above I do not know how to make a general test that will detect missing entries in the schema.

@waychal
Copy link
Contributor

waychal commented May 3, 2018

@ltalirz
url = self.get_url_prefix() + '/cifs/' + node_uuid + '/content/download'
and http://localhost:5000/api/v2/cifs/7b3113c7a9b998a8685e49e31a1cac8d2d2a997d/content/download
both are same. If test is working for this endpoint then it should also be working in browser unless file available for that node is not present or accessible and/or aiida orm is returning error.
With latest changes from develop branch, this url is working fine. Did you already fix the issue?

I guess we do not have any tests available for schema. Its better to add some.

@ltalirz
Copy link
Member Author

ltalirz commented May 3, 2018

@waychal

url = self.get_url_prefix() + '/cifs/' + node_uuid + '/content/download'
and http://localhost:5000/api/v2/cifs/7b3113c7a9b998a8685e49e31a1cac8d2d2a997d/content/download
both are same.

It seems they aren't.
Please upload a cif file to your DB and try.
If they are the same, how can the tests pass?

@waychal
Copy link
Contributor

waychal commented May 7, 2018

@ltalirz Error was in below if condition:

-        if self.custom_schema is not None and 'columns' in self.custom_schema:
+        if self.custom_schema is not None and self.__label__ in self.custom_schema['columns'].keys():
             self._default_projections = self.custom_schema['columns'][
                 self.__label__]
         else:
             self._default_projections = ['**']

in case of testing, self.custom_schema is always None so its goes to else condition and test passes.

In browser, if you have custom_schema file in common folder, if condition becomes true (self.custom_schema is not None and 'columns' in self.custom_schema) and it gives internal server error for line:
self._default_projections = self.custom_schema['columns'][self.label]
If you do not have custom_schema file in common folder, everything works fine as in that case it uses else block.

I have fixed the if condition and created PR.
Let me know if you still have queries.

@ltalirz
Copy link
Member Author

ltalirz commented May 7, 2018

@waychal Thanks a lot for investigating this and finding the problem!
It's important we get these fixes out soon, so let's do this in release_v0.12.1

@sphuber
Copy link
Contributor

sphuber commented May 9, 2018

Fixed in PR #1490

@sphuber sphuber closed this as completed May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants