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

Return JSON of result with successful status request #197

Merged
merged 4 commits into from
Dec 13, 2021

Conversation

csegarragonz
Copy link
Collaborator

@csegarragonz csegarragonz commented Dec 13, 2021

In this PR I modify the return text of the endpoint API for status requests that succeed to return a JSON string representing the result.

@csegarragonz csegarragonz self-assigned this Dec 13, 2021
@csegarragonz csegarragonz changed the title Async executed time Return executed time with successful status request Dec 13, 2021
@@ -11,6 +11,3 @@ signal:*

# TODO: Remove: There's something weird going on with MPI code I don't understand
race:faabric::scheduler::MpiWorld::*

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also remove this from here as discussed offline with jakub (don't think it is worth tagging them here).

Copy link
Collaborator

@Shillaker Shillaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing a string like this is quite fragile and a bit of a hack, plus we'll have to hard-code the output format in all the clients that consume it.

Instead it would be more robust and flexible to make the return value a JSON string dump of the result object given that it's already there. This is probably how it should have been in the first place to be honest, I don't remember why I did the strings.

Admittedly this will break all existing clients expecting those strings, but we can do a one-off update of each one and hopefully it'll be more useful long term. We should be able to use util::messageToJson.

@csegarragonz csegarragonz merged commit 5f0d60a into master Dec 13, 2021
@csegarragonz csegarragonz deleted the async-executed-time branch December 13, 2021 15:10
@Shillaker Shillaker changed the title Return executed time with successful status request Return JSON of result with successful status request Dec 13, 2021
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.

2 participants