feat: configuration file for sam cli commands #1502
Conversation
sriram-mv
force-pushed
the
config_file
branch
2 times, most recently
from
25307ea to
5cca4c3
Compare
|
Integration tests are working locally, need to investigate appveyor gates. Unrelated PR: #1501 is experiencing similar issues. |
sriram-mv
force-pushed
the
config_file
branch
2 times, most recently
from
61b79d6 to
417b857
Compare
|
open to modification of |
awood45
left a comment
awood45
left a comment
There was a problem hiding this comment.
I think this is well-formed, assuming the design stays in line. Before shipping, I'd want to see functional or integration tests which integrate config file fixtures against real commands.
|
@awood45 thanks for the review! yes those tests will be added in next series of commits. |
sanathkr
left a comment
sanathkr
left a comment
There was a problem hiding this comment.
Also, add the config file option to generate-event command as well
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
i guess this will be changed to samconfig.toml
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
- What happens if the file does not exist?
- What happens if the TOML file cannot be parsed?
There was a problem hiding this comment.
there are checks for that.
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
Should we print a error message to customers?
There was a problem hiding this comment.
changed to error, although might be polluting if they choose not to use a config file.
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
Quick line explaining what the method does?
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
Is this param a string or pathlib?
There was a problem hiding this comment.
its a string. understandable by os.path, can change to pathlib
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
How is default added to param?
samcli/cli/cli_config_file.py
Outdated
There was a problem hiding this comment.
so this os.path.abspath again here is dangerous. It is a good practice to do relative->absolute path conversion once because it's tricky getting the right cwd
- add passthroughs to the CLI command line interface via a configuration file - local defaults are set at: `.aws-sam/samconfig.toml` - This commit contains code copied and modified from https://github.com/phha/click_config_file/blob/master/click_config_file.py under MIT license
- descope environment variables - move samconfig.toml back to project root
sriram-mv
force-pushed
the
config_file
branch
from
417b857 to
6b905a6
Compare
* feat: configuration file for sam cli commands - add passthroughs to the CLI command line interface via a configuration file - local defaults are set at: `.aws-sam/samconfig.toml` - This commit contains code copied and modified from https://github.com/phha/click_config_file/blob/master/click_config_file.py under MIT license * fix: add identifier name argument instead of configuration files. * rework: change command key construction logic - descope environment variables - move samconfig.toml back to project root * docstring: TomlProvider class * fix: allow spaces on certain deploy options * fix: REGEX for parameter overrides * fix: infer config path name from ctx * fix: set abs path to `ctx.config_path` * fix: set dirname for config_path not template file name.
* feat: configuration file for sam cli commands - add passthroughs to the CLI command line interface via a configuration file - local defaults are set at: `.aws-sam/samconfig.toml` - This commit contains code copied and modified from https://github.com/phha/click_config_file/blob/master/click_config_file.py under MIT license * fix: add identifier name argument instead of configuration files. * rework: change command key construction logic - descope environment variables - move samconfig.toml back to project root * docstring: TomlProvider class * fix: allow spaces on certain deploy options * fix: REGEX for parameter overrides * fix: infer config path name from ctx * fix: set abs path to `ctx.config_path` * fix: set dirname for config_path not template file name.
4 participants
add passthroughs to the CLI command line interface via a configuration
file
local defaults are set at:
samconfig.tomlat project root.This commit contains code copied and modified from https://github.com/phha/click_config_file/blob/master/click_config_file.py under MIT license
Design doc PR is separate: design: samconfig #1503
Design doc
Write unit tests
Write/update functional tests
Write/update integration tests
make prpassesWrite documentation
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.