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

Update AiiDA REST API #878

Merged
merged 47 commits into from
Oct 31, 2017
Merged

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Oct 31, 2017

This adds a few fixes to the AiiDA REST API

  • structure endpoint can now return structure data in different formats (.xyz, .cif,...)
  • adds server info endpoint
  • hide sensitive user information

Snehal Waychal and others added 30 commits March 14, 2017 11:33
Conflicts:
	aiida/restapi/translator/base.py
will move to separate fork in the future
Merge waychal/materialscloud into aiidateam/materialscloud
Merge waychal/Materialscloud into aiidateam/materialscloud
ltalirz and others added 17 commits October 26, 2017 00:05
…r_endpoints_mcloud

Fixed issue 793 restapi server endpoints
…endpoint_mcloud

Fixed issue 777 restapi user endpoint
…cture_endpoint_mcloud

Fixed Issue 805 restapi structure endpoint
@ltalirz ltalirz requested a review from waychal October 31, 2017 18:09
@giovannipizzi giovannipizzi merged commit 7894a19 into aiidateam:develop Oct 31, 2017
@ltalirz
Copy link
Member Author

ltalirz commented Nov 1, 2017

Thanks for approving @giovannipizzi , but this was perhaps a bit fast ( @waychal still wanted to have a look).
I just tried the download feature on my machine:
http://127.0.0.1:5000/api/v2/structures/10a9728f/content/download?format=cif
and it crashed the REST API

 * Running on http://127.0.0.1:5000/ (Press CTRL+C to quit)
python(82523,0x70000f6f9000) malloc: *** mach_vm_map(size=12202731648260771840) failed (error code=3)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
Segmentation fault: 11

@giovannipizzi
Copy link
Member

Wow, a segmentation fault? Never seen one with python... Do you manage to understand which call is the cause? Is there a test for this format? (BTW, is this a small or a very big structure?)

@giovannipizzi
Copy link
Member

I just tried on my Mac and I don't get errors... probably it's a bug in the version of PyCifRW we use (btw, very old), and maybe happening for big structures? @nmounet and @merkys have been discussing upgrading pycifrw to a more recent version a few days ago, with the issue that PyCifRW changes API every new version... @nmounet, @merkys did you decide which version to upgrade to?

BTW, @waychal : I just realised that you check if the output string you get for the CIF file is exactly the same (https://github.com/aiidateam/aiida_core/blob/0e0a7c55a7db70d644b47930ed7137a8cd3cc84e/aiida/backends/tests/restapi.py#L766) However, as PyCifRW often slightly changes the format of the output (e.g. more spaces, more comments, ...) between one version and another, it would be better to just check if the file is valid even if slightly different. E.g., if there are specific substrings like _atom_site_type_symbol etc.; or by reloading the CIF file and checking, e.g., the number/type of atoms.

@ltalirz
Copy link
Member Author

ltalirz commented Nov 1, 2017

This discussion will continue in #882

@merkys
Copy link
Member

merkys commented Nov 2, 2017

@giovannipizzi: I could replace pycifrw with pycodcif (see my post in #882), however, I need a help to C-compile it on Mac. Also, this would require a migration of CifData, but interface/data structure of pycodcif is less bound to change.

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

Successfully merging this pull request may close these issues.

3 participants