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

fix(influx): validate host urls for influx config commands #18707

Merged
merged 2 commits into from
Jun 24, 2020

Conversation

jsteenb2
Copy link
Contributor

@jsteenb2 jsteenb2 commented Jun 24, 2020

Closes #18706

note: ignore white space for this PR, makes this body of work pretty small

this deprecates the old flags, but they are still accessible if using them
explictly. They are hidden from the -h message. When used the CLI prints out
a message indicating the flag is deprecated and to use the new flag instead.
@jsteenb2 jsteenb2 requested a review from sanderson June 24, 2020 19:23
@jsteenb2 jsteenb2 force-pushed the 18706/influx_config_host_parse branch from ccd406a to e44e739 Compare June 24, 2020 19:25
// the short flags will still be respected but their long form is different.
cmd.Flags().StringVar(&b.name, "name", "", "The config name (required)")
cmd.Flags().MarkDeprecated("name", "use the --config-name flag")
cmd.Flags().StringVar(&b.url, "url", "", "The host url (required)")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sanderson these are the flags that are being deprecated. They still work, but won't show up in the -h anymore

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@sanderson sanderson left a comment

Choose a reason for hiding this comment

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

LGMT!

// the short flags will still be respected but their long form is different.
cmd.Flags().StringVar(&b.name, "name", "", "The config name (required)")
cmd.Flags().MarkDeprecated("name", "use the --config-name flag")
cmd.Flags().StringVar(&b.url, "url", "", "The host url (required)")
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@jsteenb2 jsteenb2 force-pushed the 18706/influx_config_host_parse branch from e44e739 to caccb66 Compare June 24, 2020 19:35
@jsteenb2 jsteenb2 merged commit a7d29b1 into master Jun 24, 2020
@jsteenb2 jsteenb2 deleted the 18706/influx_config_host_parse branch June 24, 2020 20:11
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.

cli: creating a config should validate that the url has a schema
2 participants