Closed simu closed 7 months ago
I was looking at updating the docs, and found that https://kapitan.dev/pages/commands/kapitan_dotfile/#inventory notes that you can set inventory.inventory-path
to adjust the path to the inventory. However, this setting only applies for kapitan inventory
but not for kapitan compile
(which the docs might suggest).
Given that documentation, would it make sense to move the --inventory-path
flag to the new inventory_backend parser, and actually use .kapitan
config key inventory
for all flags for that backend, instead of having inventory-backend
as a new toplevel key in the config file?
Given that documentation, would it make sense to move the
--inventory-path
flag to the new inventory_backend parser, and actually use.kapitan
config keyinventory
for all flags for that backend, instead of havinginventory-backend
as a new toplevel key in the config file?
Yeah, it would make sense if we want to add every single argument that affects some general kapitan functionality to every command. Another approach (and what I've done for nexenio-dev) would be to group arguments by functionality and not commands. This would mean. that we don't have to check if we in compile-args
or inventory-args
and we could organize the command-parsers to include the neccessary functionality-argument-groups.
This might be a more complex (maybe breaking) change and needs to be discussed with @ademariag and @ramaro. What do you all think?
For now, I think a top level inventory_backend
key is okay.
This might be a more complex (maybe breaking) change
Thinking about it again, it would be a breaking change for sure, if we start reading argument values for existing flags from .kapitan
from a key which doesn't match the command that's executed. I'll document the current behavior for this PR and will add a note to the kapitan inventory
section in the docs to make it more clear that users may need to set the same config in multiple sections.
Perhaps we could have a global
section?
In general the inventory backend or path is not something that makes sense to have different between commands. There might be also other options that we might want to move from the subcommand to a kapitan level setting, for instance refs path, libs etc.
I think global could be a good way to start cleaning things up, while still retaining the old behaviour
Perhaps we could have a
global
section? In general the inventory backend or path is not something that makes sense to have different between commands. There might be also other options that we might want to move from the subcommand to a kapitan level setting, for instance refs path, libs etc.I think global could be a good way to start cleaning things up, while still retaining the old behaviour
Having a global
section makes sense to provide a migration path. However, I think this is best done as a separate PR since it will require a change to from_dot_kapitan()
to handle cases where a shared flag is set in both the global
and the command-specific section.
I've created a PR which implements a possible way to introduce a global
section in the config file, see #1131. Please note that I've rebased this PR onto the new PR, so we can cleanly make use of the new section in this PR.
Proposed Changes
This PR switches the inventory backend command line flag to the form
--inventory-backend=<backend name>
. Additionally, we restrict the allowed values for the flag to the known backends.To ensure the allowed values for the command line flag, and the known implementations are in sync, we introduce a dict holding the mapping from flag values to
Inventory
subclasses.Finally, we ensure that the selected backend is cached as a string in
cached.args["inventory-backend"]
. Note that this is not howcached.args
is used otherwise, but since it's unlikely that there ever will be a real commandinventory-backend
this should be fine.Docs and Tests