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

[BUG] - reject unknown CLI arguments, update docs #594

Closed
nkaretnikov opened this issue Sep 22, 2023 · 3 comments
Closed

[BUG] - reject unknown CLI arguments, update docs #594

nkaretnikov opened this issue Sep 22, 2023 · 3 comments
Labels
type: bug 🐛 Something isn't working

Comments

@nkaretnikov
Copy link
Contributor

Describe the bug

In the docs (contributing.md), we have:

python -m conda_store_server.server --standalone tests/assets/conda_store_standalone_config.py

This is wrong. It doesn't pass config to the server.

The proper way (see --help):

python -m conda_store_server.server --standalone --config <file>

Expected behavior

  1. docs are up-to-date
  2. the first invalid command is rejected, an error is raised

How to Reproduce the problem?

27d06a9

Output

No response

Versions and dependencies used.

No response

Anything else?

No response

@asmeurer
Copy link
Contributor

The real problem here is that we're using traitlets to manage the command line. Traitlets does a terrible job of handling the command line, and furthermore, this ties the public API to traitlets, which would make it harder to move away from it in the future. Also, the traitlets way of handling command line options is very verbose and difficult to work with.

I would suggest making a real command line API with argparse, which automatically handles things like invalid arguments. We could have an escape hatch to pass things through to traitlets in case that's something people are doing for backwards compatibility.

@nkaretnikov
Copy link
Contributor Author

Related: #605

@nkaretnikov
Copy link
Contributor Author

Chris and I had a chat about this. In general, Chris is OK with replacing traitlets. However, it might be tricky because we also use it for parsing configs, which is very flexible, since they can contain arbitrary Python code.

My opinion: I haven't looked into this, so hard to say, but I'd first try to make this more user-friendly while using traitlets. In general, I'm not a fan of any big rewrites unless necessary. They take a lot of time, require additional testing, and uncover new bugs that we might not know about, or have limitations that we're not currently considering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug 🐛 Something isn't working
Projects
Archived in project
Development

No branches or pull requests

3 participants