Allow missing keys in templates by default #39486

ncdc · 2017-01-05T20:33:07Z

Switch to allowing missing keys in jsonpath templates by default.

Add support for allowing/disallowing missing keys in go templates
(default=allow).

Add --allow-missing-template-keys flag to control this behavior (default=true /
allow missing keys).

Fixes #37991

@kubernetes/sig-cli-misc @kubernetes/api-reviewers @smarterclayton @fabianofranz @liggitt @pwittrock

k8s-reviewable · 2017-01-05T20:33:14Z

This change is

ncdc · 2017-01-05T20:33:27Z

Will add test-cmd tests if this looks good

ncdc · 2017-01-05T20:35:37Z

pkg/kubectl/resource_printer.go

@@ -2527,6 +2531,14 @@ func NewTemplatePrinter(tmpl []byte) (*TemplatePrinter, error) {
 	}, nil
 }

+func (p *TemplatePrinter) AllowMissingKeys(allow bool) {


I can take this bit out if we want to just apply this to the jsonpath printer for starters, as it is a slight behavior change (allow sets missingkey to zero instead of default). Or I could have allow set missingkey to default to retain the current behavior.

k8s-ci-robot · 2017-01-05T20:37:53Z

Jenkins Bazel Build failed for commit b89cf0f. Full PR test history.

The magic incantation to run this job again is @k8s-bot bazel test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ncdc · 2017-01-05T21:00:18Z

Working on fixing compilation failures and some switches that I missed. Updates coming real soon.

ncdc · 2017-01-05T21:44:08Z

@soltysh fyi

k8s-ci-robot · 2017-01-05T21:56:23Z

Jenkins Kubemark GCE e2e failed for commit e6a49a4e5d6f5149cae9a40e3b8fc2ec51742b51. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2017-01-05T22:44:27Z

Jenkins verification failed for commit 9d556b3ef5ab81c33de89989aaf53b217f4b52e8. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2017-01-05T23:11:52Z

Jenkins kops AWS e2e failed for commit 9d556b3ef5ab81c33de89989aaf53b217f4b52e8. Full PR test history.

The magic incantation to run this job again is @k8s-bot kops aws e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

smarterclayton · 2017-01-06T04:47:02Z

Looks good to me, want some opinions on naming (argument was not to use "--strict" because that's too generic) from sig cli.

ncdc · 2017-01-06T13:58:09Z

@k8s-bot test this

k8s-ci-robot · 2017-01-06T14:15:17Z

Jenkins unit/integration failed for commit f079e3ebcd62c55b307b9071ed4e457607dfba7d. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2017-01-06T15:14:26Z

Jenkins GCI GCE e2e failed for commit f079e3ebcd62c55b307b9071ed4e457607dfba7d. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ncdc · 2017-01-06T15:18:31Z

@k8s-bot gci gce e2e test this

@k8s-bot unit test this issue #39510

fabianofranz · 2017-01-06T18:17:52Z

I'd like --output-strict-templates or something else with the --output-* prefix so it's "grouped" with its context. I think the flag description must also mention it only applies to some kind(s) of --output=. Other options: --strict-missing-keys, --allow-missing-keys, --strict-template-errors, --skip-template-errors, --abort-template-errors.

soltysh · 2017-01-09T09:15:14Z

LGTM, +1 for --allow-missing-keys

ncdc · 2017-01-09T11:41:45Z

I'm not crazy about --output-strict-templates because it sounds like it's a directive saying that the command needs to output strict templates, as opposed to saying that the template input is parsed in strict mode. I do like --allow-missing-keys but maybe to be explicit it should be --allow-missing-template-keys?

soltysh · 2017-01-09T11:46:20Z

I am still struggling with readability vs brevity for this, so either of --allow-missing-keys or --allow-missing-template-keys works fine with me.

ncdc · 2017-01-09T12:03:12Z

Assuming that the default will be to allow missing keys, I would err on the side of readability for the flag's name.

…

On Mon, Jan 9, 2017 at 6:46 AM Maciej Szulik ***@***.***> wrote: I am still struggling with readability vs brevity for this, so either of --allow-missing-keys or --allow-missing-template-keys works fine with me. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#39486 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAABYoxGbe-yoMXmFgg3fKkor3A3h2BVks5rQh4ngaJpZM4LcGQg> .

fabianofranz · 2017-01-09T14:07:22Z

Assuming that the default will be to allow missing keys, I would err on the side of readability for the flag's name.

I agree, this is not something to be used too often so readability > brevity. Update the PR and let's get this in.

soltysh · 2017-01-09T14:20:58Z

Yes!

Switch to allowing missing keys in jsonpath templates by default. Add support for allowing/disallowing missing keys in go templates (default=allow). Add --allow-missing-template-keys flag to control this behavior (default=true / allow missing keys).

soltysh · 2017-01-09T16:09:03Z

pkg/kubectl/resource_printer.go

 		if err != nil {
 			return nil, false, fmt.Errorf("error parsing template %s, %v\n", formatArgument, err)
 		}
+		templatePrinter.AllowMissingKeys(allowMissingTemplateKeys)


How about extending NewTemplatePrinter with that new flag since we're passing it every time we use it?

soltysh · 2017-01-09T16:09:20Z

Neat, otherwise LGTM.

fabianofranz · 2017-01-09T16:32:18Z

LGTM

k8s-github-robot · 2017-01-10T22:33:09Z

Automatic merge from submit-queue (batch tested with PRs 39486, 37288, 39477, 39455, 39542)

ncdc · 2017-01-12T13:54:24Z

Do we want to cherrypick this to 1.5?

soltysh · 2017-01-12T17:35:08Z

I say - yes.

k8s-cherrypick-bot · 2017-01-12T17:39:28Z

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

…39895-#39038-upstream-release-1.5 Automatic merge from submit-queue Automated cherry pick of #39486 #39895 #39038 Cherry pick of #39486 #39895 #39038 on release-1.5. #39486: Allow missing keys in templates by default #39895: Fix expected error text #39038: Fix kubectl get -f <file> -o <nondefault printer>

ncdc added the sig/cli Categorizes an issue or PR as relevant to SIG CLI. label Jan 5, 2017

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 5, 2017

ncdc commented Jan 5, 2017

View reviewed changes

k8s-github-robot assigned adohe-zz Jan 5, 2017

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jan 5, 2017

ncdc force-pushed the allow-missing-keys-in-templates branch from b89cf0f to e6a49a4 Compare January 5, 2017 21:16

ncdc added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jan 5, 2017

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jan 5, 2017

ncdc force-pushed the allow-missing-keys-in-templates branch from e6a49a4 to 9d556b3 Compare January 5, 2017 21:46

ncdc force-pushed the allow-missing-keys-in-templates branch from 9d556b3 to f079e3e Compare January 6, 2017 01:11

ncdc force-pushed the allow-missing-keys-in-templates branch from f079e3e to 80c5cd8 Compare January 9, 2017 15:39

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 9, 2017

soltysh reviewed Jan 9, 2017

View reviewed changes

fabianofranz added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 9, 2017

k8s-github-robot merged commit 9ef9630 into kubernetes:master Jan 10, 2017

ncdc added the cherrypick-candidate label Jan 12, 2017

k8s-cherrypick-bot removed the cherrypick-candidate label Jan 12, 2017

ncdc added this to the v1.5 milestone Jan 12, 2017

ncdc added the cherrypick-candidate label Jan 12, 2017

ncdc mentioned this pull request Jan 16, 2017

Automated cherry pick of #39486 #39895 #39038 #39954

Merged

saad-ali added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jan 20, 2017

saad-ali removed the cherrypick-candidate label Feb 9, 2017

ncdc deleted the allow-missing-keys-in-templates branch February 13, 2017 17:23

mengqiy mentioned this pull request Jun 14, 2017

jsonpath: missing `left' in filter has confusing error message and fails unexpectedly #45546

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow missing keys in templates by default #39486

Allow missing keys in templates by default #39486

ncdc commented Jan 5, 2017 •

edited

Loading

k8s-reviewable commented Jan 5, 2017

ncdc commented Jan 5, 2017

ncdc Jan 5, 2017 •

edited

Loading

k8s-ci-robot commented Jan 5, 2017

ncdc commented Jan 5, 2017

ncdc commented Jan 5, 2017

k8s-ci-robot commented Jan 5, 2017

k8s-ci-robot commented Jan 5, 2017

k8s-ci-robot commented Jan 5, 2017

smarterclayton commented Jan 6, 2017

ncdc commented Jan 6, 2017

k8s-ci-robot commented Jan 6, 2017

k8s-ci-robot commented Jan 6, 2017

ncdc commented Jan 6, 2017

fabianofranz commented Jan 6, 2017

soltysh commented Jan 9, 2017

ncdc commented Jan 9, 2017

soltysh commented Jan 9, 2017

ncdc commented Jan 9, 2017 via email

fabianofranz commented Jan 9, 2017

soltysh commented Jan 9, 2017

soltysh Jan 9, 2017

ncdc Jan 9, 2017

soltysh Jan 9, 2017

soltysh commented Jan 9, 2017

fabianofranz commented Jan 9, 2017

k8s-github-robot commented Jan 10, 2017

ncdc commented Jan 12, 2017

soltysh commented Jan 12, 2017

k8s-cherrypick-bot commented Jan 12, 2017

Allow missing keys in templates by default #39486

Allow missing keys in templates by default #39486

Conversation

ncdc commented Jan 5, 2017 • edited Loading

k8s-reviewable commented Jan 5, 2017

ncdc commented Jan 5, 2017

ncdc Jan 5, 2017 • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 5, 2017

ncdc commented Jan 5, 2017

ncdc commented Jan 5, 2017

k8s-ci-robot commented Jan 5, 2017

k8s-ci-robot commented Jan 5, 2017

k8s-ci-robot commented Jan 5, 2017

smarterclayton commented Jan 6, 2017

ncdc commented Jan 6, 2017

k8s-ci-robot commented Jan 6, 2017

k8s-ci-robot commented Jan 6, 2017

ncdc commented Jan 6, 2017

fabianofranz commented Jan 6, 2017

soltysh commented Jan 9, 2017

ncdc commented Jan 9, 2017

soltysh commented Jan 9, 2017

ncdc commented Jan 9, 2017 via email

fabianofranz commented Jan 9, 2017

soltysh commented Jan 9, 2017

soltysh Jan 9, 2017

Choose a reason for hiding this comment

ncdc Jan 9, 2017

Choose a reason for hiding this comment

soltysh Jan 9, 2017

Choose a reason for hiding this comment

soltysh commented Jan 9, 2017

fabianofranz commented Jan 9, 2017

k8s-github-robot commented Jan 10, 2017

ncdc commented Jan 12, 2017

soltysh commented Jan 12, 2017

k8s-cherrypick-bot commented Jan 12, 2017

ncdc commented Jan 5, 2017 •

edited

Loading

ncdc Jan 5, 2017 •

edited

Loading