Change id field passed to JobInfo from int to str
#30
Conversation
fixes aiidateam#29 The id field of JobInfo is expecting a str. In aiidateam#24 when parsing JSON output of `hq job list` the json loads will use the int for the id parsed directly. Wrong type causes the subtle issue that when job is waiting it not get into QUEUED state, but immediatly finished and get nothing to parse from output.
for more information, see https://pre-commit.ci
| @@ -228,7 +228,9 @@ def _parse_joblist_output(self, retval: int, stdout: str, stderr: str) -> list: | |||
| job_info_list = [] | |||
| for hq_job_dict in hq_job_info_list: | |||
| job_info = JobInfo() | |||
| job_info.job_id = hq_job_dict["id"] | |||
| job_info.job_id = str( | |||
There was a problem hiding this comment.
This bug cost me couple of hours to locate it. Maybe in the aiida-core it can provide type check to avoid scheduler passing wrong datatype? @sphuber
There was a problem hiding this comment.
The JobInfo is part of the many data classes that are used that are defined as the custom DefaultFieldsAttributeDict. They really should be replaced with a dataclass but not sure about backwards compatibility. Also not sure there is much we can do about this now as long as it remains a DefaultFieldsAttributeDict.
That being said, how exactly did this bug manifest? I think it should automatically be cast into str somewhere. If not, it might just be a bug in aiida-core.
There was a problem hiding this comment.
Thanks @sphuber I dig a bit more into source of the issue (aiidateam/aiida-core#6542), but didn't able to find a good solution.
|
Thanks @t-reents for reviewing. I'll merge this as a workaround. |
fixes #29
The id field of JobInfo is expecting a str. In #24 when parsing JSON output of
hq job listthe json loads will use the int for the id parsed directly. Wrong type causes the subtle issue that when job is waiting it not get into QUEUED state, but immediatly finished and get nothing to parse from output.