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

add parameters to config #23

Merged
merged 4 commits into from
Sep 25, 2017
Merged

add parameters to config #23

merged 4 commits into from
Sep 25, 2017

Conversation

ScottABrown
Copy link
Contributor

@ScottABrown ScottABrown commented Sep 23, 2017

This commit adds parameters including a no-prompt option for config(), to facilitate Docker container setup.

  • This makes the current prompted settings configurable by options to "aardvark config".
  • I think the aardvark-role, swag-bucket and db-uri are likely more common, so they have short options available. num-threads, phantom and no-prompt only offer long options.
  • I suspect the region and a couple other config settings should be configurable but that can be a separate commit. I considered '-r' as short for aardvard-role but wanted to reserve it for the possibility of a region option, so I used '-a' as short for aardvark-role instead.
  • For consistency/simplicity, to allow a no-prompt option without requiring some parameters be specified this adds nominal default values for the swag bucket name and the aardvark role: DEFAULT_SWAG_BUCKET = 'swag-data' and DEFAULT_AARDVARK_ROLE = 'Aardvark'.
  • The config() function now raises an exception if the phantomjs executable can't be located at config() time and isn't specified either by parameter or user input.
  • Fixed some formatting issues in the config file writes (braces in format strings and a couple of newlines).

@mcpeak
Copy link
Contributor

mcpeak commented Sep 25, 2017

Great work, thanks!

@mcpeak mcpeak merged commit c31f535 into Netflix-Skunkworks:master Sep 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants