Move last_jobinfo from JSON-serialized string to dictionary #3651
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sphuber
previously approved these changes
Dec 12, 2019
There was a problem hiding this comment.
Thanks @giovannipizzi looking good, I would just turn the loaders into classmethods
sphuber
requested changes
Dec 12, 2019
giovannipizzi
force-pushed
the
fix_3649_last_jobinfo
branch
2 times, most recently
from
756e1e9 to
065ec84
Compare
sphuber
force-pushed
the
fix_3649_last_jobinfo
branch
from
065ec84 to
852136f
Compare
For historical reasons, this field was stored as a JSON-serialized string in the `last_jobinfo` attribute of a CalcJob. However, this is cumbersome and makes querying very hard. We now replace this with a dictionary, thanks to new commands to get directly a dictionary (with serialized fields, so that the dictionary is JSON-serializable). These (and existing) methods of the JobInfo class are now also tested. Finally, the attribute key has been renamed from `last_jobinfo` to `last_job_info`, for consistency with the key `detailed_job_info` introduced in aiidateam#3639. By changing the type of the content, the field is anyway not directly usable as before in scripts, so changing the name is not an additional issue. This should not give a real backward- incompatibility problem, since this field was there mostly for debugging reasons.
sphuber
force-pushed
the
fix_3649_last_jobinfo
branch
from
852136f to
4b9f86c
Compare
sphuber
approved these changes
Dec 13, 2019
|
Rebasing and squashing seems to have done it, the tests passed. Or maybe it was just luck/random again. We will see. Thanks for the PR in any case @giovannipizzi ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For historical reasons, this field was stored as a JSON-serialized
string in the
last_jobinfoattribute of a CalcJob. However, thisis cumbersome and makes querying very hard.
We now replace this with a dictionary, thanks to new commands to
get directly a dictionary (with serialized fields, so that the
dictionary is JSON-serializable).
These (and existing) methods of the JobInfo class are now also tested.
Finally, the attribute key has been renamed from
last_jobinfotolast_job_info, for consistency with the keydetailed_job_infointroduced in #3639. By changing the type of the content, the field is
anyway not directly usable as before in scripts, so changing the name
is not an additional issue.
This should not give a real backward-incompatibility problem, since
this field was there mostly for debugging reasons.
Fixes #3649