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

Make --network accept a file argument and add a --flake flag #1480

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

Conversation

talyz
Copy link
Contributor

@talyz talyz commented Oct 21, 2021

To make it possible to keep multiple network definitions in the same directory, make the --network flag accept a file in addition to a directory. Also, add a corresponding --flake flag and create short flags for both (-n and -f respectively).

@talyz talyz force-pushed the master branch 2 times, most recently from 2809295 to 0ac23a7 Compare October 21, 2021 12:12
@talyz talyz mentioned this pull request Nov 23, 2021
16 tasks
@roberth roberth added this to In progress in 2.0 Release via automation Nov 23, 2021
@roberth
Copy link
Member

roberth commented Nov 23, 2021

This needs documentation and tests.

We have unit tests in tests.py and the tests directory, or NixOS/nixpkgs#142035 could be added here for end to end tests.

Not sure if eval-machine-info needs to use the name, instead of nixopsConfigurations.default. Perhaps that should be independent and part of the flake fragment: --flake .#other-deployment.

Documentation can be updated in doc/manual/nixops.rst

@talyz
Copy link
Contributor Author

talyz commented Nov 23, 2021

This needs documentation and tests.

We have unit tests in tests.py and the tests directory, or NixOS/nixpkgs#142035 could be added here for end to end tests.

I'm not sure how to add this to the unit tests - it doesn't really feel suited for it. The NixOS test could be updated for it, though.

Not sure if eval-machine-info needs to use the name, instead of nixopsConfigurations.default. Perhaps that should be independent and part of the flake fragment: --flake .#other-deployment.

I'm not really sure what this is referring to in relation to this change.

Documentation can be updated in doc/manual/nixops.rst

I've added some basic parameter documentation now and removed the references I could find to the previous -n flag. There's more to document and more in the documentation to update, but I think it makes more sense to do a bigger rewrite when we know how the interface looks - should deployments and the legacy backend be removed or should legacy deployments be fixed instead? As I see it, the prior alternative would require more substantial documentation updates (and be a lot of work in general), while the latter would give us a confusing UX with a lot of old cruft.

network_path: str = ""
flake_path: str = ""

if args.network_path is not None:

Choose a reason for hiding this comment

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

Suggested change
if args.network_path is not None:
if args.network_path is not None and args.flake_path is not None:
raise ValueError("You can only provide one of either -f or -n")
if args.network_path is not None:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is already covered by argparse, since --network and --flake are put into a mutually exclusive group.

To make it possible to keep multiple network definitions in the same
directory, make the `--network` flag accept a file in addition to a
directory. Also, add a corresponding `--flake` flag and create short
flags for both (`-n` and `-f` respectively).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
2.0 Release
In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants