Transform result to dict before returning from Job.result #944
Conversation
| @@ -375,7 +375,11 @@ def result(self, wait=True, cadence=5, verbose=False): | |||
| time.sleep(cadence) | |||
| if verbose: | |||
| logging.info(".") | |||
| return self._job_client.result(self.job_id) | |||
| results = self._job_client.result(self.job_id) | |||
| if isinstance(results, str): | |||
There was a problem hiding this comment.
I could be overthinking things here, but would it be better to handle this within the job client (i.e. here)?
There was a problem hiding this comment.
It looks like the return is already loaded into a dictionary in that line, but when I use save_result inside my remote program, job.result() returns me a string representation of a dictionary
There was a problem hiding this comment.
@IceKhan13 why would the return object in the deleted line ever be a string here? _job_client.result() returns a decoded object here
Is there another client which may be returning encoded strings to job.result besides GatewayJobClient?
There was a problem hiding this comment.
Hmm, good question. It should be decoded object (dict). I do not know why it's returning string. Worth digging into it
|
@caleb-johnson can you update code to pass lints and we can merge this :) |
I can do that, but I am not really satisfied that this is the right fix. I do not understand why we need to decode the string here, when it is already being done in the GatewayJobClient. Doing what I have in this PR seems redundant. Is there another client being used I am not aware of? Do you understand what is causing these JSON strings to make it through the |
|
Something looks a bit off with the commits... |
caleb-johnson
force-pushed
the
result-dict
branch
from
097d433
to
2fe6af9
Compare
Thanks, I cleaned it up. I made a mess by merging main into this branch :P Going to take a closer look at this now that Aki solved the other problem with results |
Co-authored-by: Iskandar Sitdikov <IceKhan13@users.noreply.github.com>
caleb-johnson
force-pushed
the
result-dict
branch
from
2fe6af9
to
d446948
Compare
|
@caleb-johnson thing is we have DB schema in following format so when it is returned from DB json looks like this {
"id": "42",
"status": "SUCCEEDED",
"results": "{'bla-bla': 'bla-bla'}"
}therefore we need to do another decoding on It can be fixed by changing https://github.com/Qiskit-Extensions/quantum-serverless/blob/main/gateway/api/models.py#L94 to JsonField, probably. Or even better, by creating custom serializer which will decode results field before returning back to user https://github.com/Qiskit-Extensions/quantum-serverless/blob/main/gateway/api/serializers.py#L23 |
| try: | ||
| results = json.loads(results, cls=QiskitObjectsDecoder) | ||
| except json.JSONDecodeError: | ||
| pass |
There was a problem hiding this comment.
can we log something here?
logger.warning("Error during results decoding. Details: %s", exception)
|
@IceKhan13 , as discussed, we can just merge this patch and follow up with a better fix in a following PR. Added a warning when the string isn't interpretable by json, as requested. Ready for final review/approval |
Fixes #942