Support loading network configuration from file#1227
Support loading network configuration from file#1227apoorvdarshan wants to merge 1 commit intoapple:mainfrom
Conversation
|
@apoorvdarshan Thank you for the contribution! @katiewasnothere Was the |
|
I was drafting this comment when I noticed things were changing a bit. However, I believe the feedback is still valid and may be worth discussing. Instead of making the flags mutually exclusive, I think we should check whether a CLI option is explicitly set and prefer it over the value from the config file when both are provided (intentionally or accidentally)? It seems to be a more common and flexible pattern that improves usability. |
Add --config option to `container-network-vmnet start` that accepts a path to a JSON configuration file. CLI flags override values from the config file when both are provided. Introduces NetworkConfigurationFile as an intermediate Codable struct that is decoded from the JSON file and converted to a full NetworkConfiguration via toNetworkConfiguration(id:).
efd10f6 to
38f68d7
Compare
|
Thanks for the feedback! @katiewasnothere Good catch — I've moved the @saehejkang Agreed, the override approach is better. I've updated the implementation so that CLI flags override config file values when both are provided, rather than making them mutually exclusive. |
|
@apoorvdarshan Thanks! I think we'll need until a little later next week to work through some design stuff related to this. We may need a change or two but what you have is in the direction of where things are going. On these helper executables, we're wanting to eliminate the options in favor of either a This helps make it so that the plugin-specific details of runtimes and networks are the concern of the CLI and service plugins, and those details are opaque to the containers and networks services in the API server; they just need to create the entity directories that the plugins understand. |
|
@jglogan Thanks for the context! That makes sense — having helper executables take a |
Summary
--configoption tocontainer-network-vmnet startthat accepts a path to a JSON configuration file, closing [Request]: Support loading network configuration from file #1047NetworkConfigurationFileas an intermediateCodablestruct that is decoded from JSON and converted to a fullNetworkConfigurationviatoNetworkConfiguration(id:)--modeand--variantchanged to optionals so explicit CLI usage can be detected and preferred over config file valuesTest plan
NetworkConfigurationFile: minimal decode, complete decode, invalid subnet, invalid IDNetworkConfigurationTesttests continue to passswift buildsucceedsswift test --filter NetworkConfigurationTestpasses (9/9 tests)