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

Fix JSON output issues #1600

Merged
merged 8 commits into from
Apr 7, 2022
Merged

Conversation

Klaim
Copy link
Member

@Klaim Klaim commented Mar 29, 2022

This fixes several issues:

  • potential unhandled exception thrown when a mix of args ends up calling json_up() but the json is empty;
  • replaced explicit calls to json printing to the output by automatic printing to the output at the end of the program (after main()) - this is mainly to prevent the json output to end up invalid if called more than once:
{ // output by the first part of the operation
    "success": true,
}
{
   // some other values from the rest of the operation
}

This is invalid json because there is more than one root objects and makes it difficult or impossible to check the json ouput in tests.

  • refactored a bit and added a few checks;
  • added a way to cancel the print of json log from libmamba, used in mamba which already output the right json;

Note that these issues were spotted while making tests for #1577 , they are hard to reproduce otherwise.

@wolfv
Copy link
Member

wolfv commented Mar 29, 2022

Looks great!

@wolfv
Copy link
Member

wolfv commented Mar 29, 2022

For the linting you can just run pre-commit run --all.

@Klaim Klaim force-pushed the klaim/fix-json-output branch from e8d70ec to 0d2796d Compare March 29, 2022 14:05
@Klaim Klaim marked this pull request as draft March 31, 2022 09:12
@Klaim Klaim force-pushed the klaim/fix-json-output branch 4 times, most recently from aee4273 to df3ec58 Compare April 1, 2022 14:06
Klaim added 4 commits April 1, 2022 18:34
From now on the json output will be automatically printed at the end of the program
(after `main()` call) instead of explicitly invoked.
This is to prevent potential multiple prints in the output which then would be
invalid JSON.
@Klaim Klaim force-pushed the klaim/fix-json-output branch from df3ec58 to e97272f Compare April 1, 2022 16:36
@Klaim Klaim force-pushed the klaim/fix-json-output branch from e97272f to bef2935 Compare April 6, 2022 14:00
@Klaim Klaim marked this pull request as ready for review April 6, 2022 14:44
@JohanMabille JohanMabille merged commit 048cb67 into mamba-org:master Apr 7, 2022
@Klaim Klaim deleted the klaim/fix-json-output branch December 19, 2024 02:29
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