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

Allow specifying the expected return code instead of erroring on non-zero results #10

Closed
ssbarnea opened this issue Jan 26, 2023 · 5 comments

Comments

@ssbarnea
Copy link
Sponsor Contributor

While documenting different executions, we might really need to execute commands that do not return 0 exit code. Current version of the plugin does raise a warning, which is converted to an error when running in strict mode.

Looking at how others did this, we can take a look at https://pythonhosted.org/sphinxcontrib-programoutput/#error-handling which allows users to mention expected return code.

@pawamoy
Copy link
Owner

pawamoy commented Jan 26, 2023

Agreed. I didn't know Sphinx had such a plugin 🙂

@pawamoy
Copy link
Owner

pawamoy commented Jan 27, 2023

WDYT about an expect option that accepts an integer as value?

```bash exec="true" expect="1"
program-that-exits-with-1
```

Also, should we accept other values for Python code blocks? Something like expect="ValueError"?
I don't see a use-case for expecting particular Python exceptions: one can always just use a regular python/pycon code block without executing it, showing what the exception looks like.
Or even execute a code block that catches the exception and simply prints the formatted traceback.

@ssbarnea
Copy link
Sponsor Contributor Author

For the sake of consistency with other tools, please use returncode and not expect as that can have lots of meanings, the library or the expected output?

returncode is already used by https://docs.python.org/3/library/subprocess.html and by https://pythonhosted.org/sphinxcontrib-programoutput/#error-handling

I think that even an exception would translate to an exit code, so no need to extra complicate it.

$ python -c "import foo"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'foo'
FAIL: 1

Basically python will return exit code "1" when an exception is raised and I would recommend using the same approach for python blocks. Think about other code blocks, they should all run as subprocess for safety reasons anyway, so "returncode" is the portable solution.

I will also want to add an option to hide stderr like I had with sphinx, for now I had to add 2>/dev/null to the commands to hide the noise from it. Making a PR for this would clearly be a good opportunity for me to learn some of the internals.

@pawamoy
Copy link
Owner

pawamoy commented Jan 27, 2023

Thanks for the feedback! I'll go with returncode then.

About execution of Python code blocks: they're currently executed in the main process, with an isolated globals dictionary each time. I didn't want to spawn a Python subprocess for each executed Python code block, the perf would be disastrous. But we can consider adding an option to let the user decide if they want to run a Python code block in the main process or in a subprocess.

pawamoy added a commit that referenced this issue Jan 27, 2023
@pawamoy
Copy link
Owner

pawamoy commented Jan 27, 2023

Implemented (see commit above) 🙂

@pawamoy pawamoy closed this as completed Jan 27, 2023
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

No branches or pull requests

2 participants