Skip to content

Fix handling for unknown config variable in pyodide config get #200

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix handling for unknown config variable in pyodide config get #200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions pyodide_build/cli/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import sys

import typer

from pyodide_build.build_env import (
Expand Down Expand Up @@ -47,7 +49,7 @@ def get_config(
configs = _get_configs()

if config_var not in configs:
typer.echo(f"Config variable {config_var} not found.")
typer.Exit(1)
print(f"Config variable {config_var} not found.", file=sys.stderr)
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of changing typo.echo to print? If the goal is to avoid using typer APIs we might change it everywhere.

Copy link
Member Author

@hoodmane hoodmane May 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The point is to print to stderr. I was unable to figure out how to do that with typer.echo()

Copy link
Member

Choose a reason for hiding this comment

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

The docs at https://typer.tiangolo.com/tutorial/printing/?h=stderr#printing-to-standard-error say that we need to use stderr=True, but it has to be done at the rich.console.Console() level. Maybe we should set that somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that and didn't fix it in ~10 minutes or so of messing around. I think it matters much more that the print goes to stderr then that it goes through rich. The same looking text shows up on the console at the end either way.

raise typer.Exit(1)
Copy link
Member

Choose a reason for hiding this comment

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

👍


typer.echo(configs[config_var])
24 changes: 21 additions & 3 deletions pyodide_build/tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
)
from pyodide_build.config import PYODIDE_CLI_CONFIGS

runner = CliRunner()
runner = CliRunner(mix_stderr=False)

RECIPE_DIR = Path(__file__).parent / "recipe" / "_test_recipes"

Expand Down Expand Up @@ -247,6 +247,7 @@ def test_config_list(dummy_xbuildenv):
"list",
],
)
assert_runner_succeeded(result)

envs = result.stdout.splitlines()
keys = [env.split("=")[0] for env in envs]
Expand All @@ -264,10 +265,27 @@ def test_config_get(cfg_name, env_var, dummy_xbuildenv):
cfg_name,
],
)
assert_runner_succeeded(result)

assert result.stdout.strip() == build_env.get_build_flag(env_var)


def test_config_unknown(dummy_xbuildenv):
result = runner.invoke(
config.app,
[
"get",
"unknown-variable",
],
)

assert result.exit_code == 1
assert result.stdout.strip() == ""
# It should be "Config variable unknown-variable not found." but the output went missing somehow??
# It successfully finds it if we print it to stdout.
assert result.stderr.strip() == ""


def test_create_zipfile(temp_python_lib, temp_python_lib2, tmp_path):
from zipfile import ZipFile

Expand All @@ -285,8 +303,8 @@ def test_create_zipfile(temp_python_lib, temp_python_lib2, tmp_path):
str(output),
],
)
assert_runner_succeeded(result)

assert result.exit_code == 0, result.stdout
assert "Zip file created" in result.stdout
assert output.exists()

Expand Down Expand Up @@ -315,8 +333,8 @@ def test_create_zipfile_compile(temp_python_lib, temp_python_lib2, tmp_path):
"--pycompile",
],
)
assert_runner_succeeded(result)

assert result.exit_code == 0, result.stdout
assert "Zip file created" in result.stdout
assert output.exists()

Expand Down
Loading