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 config flags to specify image values, closes #1181 #1182

Merged
merged 6 commits into from
Oct 14, 2018

Conversation

andrewfrench
Copy link
Contributor

@andrewfrench andrewfrench commented Oct 12, 2018

The Problem/Issue/Bug:

#1181: Giving troubleshooting/debugging instructions can be complicated when it involves changing web/db/dba image values because it requires editing .ddev/config.yaml.

How this PR Solves The Problem:

Adds several configuration flags to ddev config that will set the images being used:

  • --web-image
  • --db-image
  • --dba-image

To unset these custom image configurations, the following flags have been added:

  • --web-image-default
  • --db-image-default
  • --dba-image-default

And the following flag has been added to unset all custom image configurations in one go:

  • --image-defaults

Some image reference getters have been added to versions.go to standardize things.

Manual Testing Instructions:

Provide custom image names to ddev config:

  • ddev config --web-image my-web-image
  • ddev config --db-image my-db-image:v1.2.3
  • ddev config --dba-image my-dba-image:latest

And combinations of the above. Ensure after each execution that .ddev/config.yaml has been updated to reflect the updated image, then test unsetting image values:

  • ddev config --web-image-default
  • ddev config --db-image-default
  • ddev config --dba-image-default
  • ddev config --image-defaults

Automated Testing Overview:

Automated tests have been expanded to test the new flags.

Related Issue Link(s):

#1181

@andrewfrench andrewfrench added this to the v1.4.0 milestone Oct 12, 2018
@andrewfrench andrewfrench self-assigned this Oct 12, 2018
@rfay
Copy link
Member

rfay commented Oct 12, 2018

Thanks for this!

One thought - although I do expect --db-image to be the correct technique, it's "dbimage" (and friends) that we use in the config.yaml. Should we use reasonable flags or go for consistency with the config.yaml?

@andrewfrench
Copy link
Contributor Author

andrewfrench commented Oct 12, 2018

I considered both, and I believe --db-image is the correct choice for readability/usability reasons, and because it matches the standard of the other flags (except --projectname and --projecttype, which I'd argue should probably be deprecated/hidden and replaced with --project-name and --project-type for the same reasons).

Edit: I'll actually just go ahead and do that here, it's a small change.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

I read this through and could find nothing objectionable. I tried out a number of the options, and tried out the deprecated options that work fine still.

Thanks so much for taking ownership of this stuff and making it so much better. I do wish we didn't have to just keep adding code, but it seems to be the way of cobra.

ddev config -h is enough to choke any horse now! And I guess I'm ok with that. I always just use ddev config and click through anyway. I imagine most people do. I think making sure that ddev config can do anything one would want to do from the command line is the right thing.

@andrewfrench andrewfrench merged commit 920a32f into ddev:master Oct 14, 2018
@andrewfrench andrewfrench deleted the 2018-10-12_container-flags branch October 14, 2018 00:27
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