search

conda-incubator / conda-store

Data science environments, for collaboration. ✨
https://conda.store
BSD 3-Clause "New" or "Revised" License
143 stars 46 forks source link

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

Open nkaretnikov opened 12 months ago

nkaretnikov commented 12 months ago

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?

27d06a9444e8f8da514391ffabaa972f73975ee9

Output

No response

Versions and dependencies used.

No response

Anything else?

No response

asmeurer commented 12 months ago

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 commented 11 months ago

Related: https://github.com/conda-incubator/conda-store/issues/605

nkaretnikov commented 11 months ago

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.