Implement human readable output for kn route describe #643

navidshaikh · 2020-02-07T12:22:19Z

/lint

Fixes #205

Proposed Changes

Keeps the machine readable output
Align the command description text, check for single argument, reported error messages and unit tests in service, revision, route, source binding describe commands

Default output:

➜  client git:(pr/describe-route) ./kn route describe foo
Name:       foo
Namespace:  default
Age:        2m
URL:        http://foo.default.apps.ci-ln-dr6jjit-d5d6b.origin-ci-int-aws.dev.rhcloud.com
Service:    foo

Traffic Targets:  
   40%  foo-zxjzw-1 #v1
        URL:  http://v1-foo.default.apps.ci-ln-dr6jjit-d5d6b.origin-ci-int-aws.dev.rhcloud.com
   60%  foo-bhjrx-2 #v2
        URL:  http://v2-foo.default.apps.ci-ln-dr6jjit-d5d6b.origin-ci-int-aws.dev.rhcloud.com

Conditions:  
  OK TYPE                  AGE REASON
  ++ Ready                 13s 
  ++ AllTrafficAssigned     2m 
  ++ IngressReady          13s

Verbose output:

➜  client git:(pr/describe-route) ./kn route describe svc -v
Name:         svc
Namespace:    default
Labels:       serving.knative.dev/service=svc
Annotations:  serving.knative.dev/creator=kube:admin
              serving.knative.dev/lastModifier=kube:admin
Age:          11m
URL:          http://svc.default.apps.ci-ln-dr6jjit-d5d6b.origin-ci-int-aws.dev.rhcloud.com
Service:      svc (serving.knative.dev/v1alpha1)

Traffic Targets:  
  100%  @latest (svc-fkvpz-1)

Conditions:  
  OK TYPE                  AGE REASON
  ++ Ready                 10m 
  ++ AllTrafficAssigned    10m 
  ++ IngressReady          10m

knative-prow-robot

@navidshaikh: 1 warning.

In response to this:

/lint

Fixes #205

Proposed Changes

Keeps the machine readable output
Align the command description text
Add simple e2e test in basic workflow test

Default output:

➜  client git:(pr/describe-route) ./kn route describe foo
Name:       foo
Namespace:  default
Age:        1d
URL:        http://foo.default.example.com

Owner References:  
 Service:  foo

Traffic Targets:  
  60%  foo-tcvhr-4
  40%  foo-rnlnj-3 #v3
       URL:  http://v3-foo.default.example.com

Conditions:  
 OK TYPE                  AGE REASON
 ++ Ready                  1d 
 ++ AllTrafficAssigned     1d 
 ++ IngressReady           1d

Verbose output:

➜  client git:(pr/describe-route) ./kn route describe svc -v  
Name:         svc
Namespace:    default
Labels:       serving.knative.dev/service=svc
Annotations:  serving.knative.dev/creator=minikube-user
             serving.knative.dev/lastModifier=minikube-user
Age:          1d
URL:          http://svc.default.example.com

Owner References:  
 Service:  svc (serving.knative.dev/v1alpha1)

Traffic Targets:  
 100%  @latest (svc-qxdyq-5)

Conditions:  
 OK TYPE                  AGE REASON
 ++ Ready                  1d 
 ++ AllTrafficAssigned     1d 
 ++ IngressReady           1d

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.

pkg/kn/commands/route/describe.go

navidshaikh · 2020-02-07T12:41:06Z

/lint

knative-prow-robot

@navidshaikh: 0 warnings.

In response to this:

/lint

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.

rhuss · 2020-02-07T14:01:36Z

@navidshaikh there's a compile error:

# knative.dev/client/test/e2e [knative.dev/client/test/e2e.test]
test/e2e/route_test.go:79:22: (*e2eTest).routeDescribe redeclared in this block
	previous declaration at test/e2e/basic_workflow_test.go:148:6

rhuss · 2020-02-07T19:27:54Z

The flaky test should be fixed with #644

rhuss · 2020-02-07T19:28:01Z

/retest

rhuss

looks good (as always ;-) but I would not output an owner reference in general: Print out a service if attached or nothing (see inline).

Otherwise, some minor nits.

docs/cmd/kn_revision_describe.md

pkg/kn/commands/route/describe.go

pkg/kn/commands/source/binding/describe.go

rhuss · 2020-02-07T20:40:32Z

/retest

maximilien

LGTM

knative-prow-robot · 2020-02-07T22:17:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: maximilien, navidshaikh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [maximilien,navidshaikh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

- Keeps the machine readable output - Align the command description text, check for single argument, reported error messages and unit tests in service, revision, route, source binding describe commands

rhuss · 2020-02-10T08:11:05Z

/retest

navidshaikh · 2020-02-10T09:09:25Z

/retest

boskos

pkg/kn/commands/route/describe.go

rhuss

looks good to me with a minor question whether to alway sprint a "Service"

rhuss · 2020-02-10T10:07:09Z

/retest

knative-metrics-robot · 2020-02-10T11:22:57Z

The following is the coverage report on the affected files.
Say /test pull-knative-client-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/kn/commands/route/describe.go	78.3%	89.5%	11.2
pkg/kn/commands/service/describe.go	80.0%	79.8%	-0.2

navidshaikh · 2020-02-10T13:00:14Z

/retest

rhuss · 2020-02-10T15:04:00Z

/retest

rhuss · 2020-02-10T15:45:12Z

thanks !

/lgtm

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 7, 2020

knative-prow-robot reviewed Feb 7, 2020

View reviewed changes

pkg/kn/commands/route/describe.go Outdated Show resolved Hide resolved

knative-prow-robot requested review from mattmoor and rhuss February 7, 2020 12:22

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 7, 2020

navidshaikh force-pushed the pr/describe-route branch from fdf798c to 5208fee Compare February 7, 2020 12:40

knative-prow-robot reviewed Feb 7, 2020

View reviewed changes

navidshaikh force-pushed the pr/describe-route branch from 5208fee to 67f7e49 Compare February 7, 2020 14:55

rhuss requested changes Feb 7, 2020

View reviewed changes

docs/cmd/kn_revision_describe.md Outdated Show resolved Hide resolved

pkg/kn/commands/route/describe.go Outdated Show resolved Hide resolved

pkg/kn/commands/route/describe.go Outdated Show resolved Hide resolved

pkg/kn/commands/source/binding/describe.go Outdated Show resolved Hide resolved

maximilien approved these changes Feb 7, 2020

View reviewed changes

Implement human readable output for kn route describe

ec9fa12

- Keeps the machine readable output - Align the command description text, check for single argument, reported error messages and unit tests in service, revision, route, source binding describe commands

navidshaikh force-pushed the pr/describe-route branch 2 times, most recently from 20e77bf to 3bf2396 Compare February 10, 2020 05:48

Dont print separate section for owner references

0878b23

navidshaikh force-pushed the pr/describe-route branch from 3bf2396 to 0878b23 Compare February 10, 2020 06:51

rhuss reviewed Feb 10, 2020

View reviewed changes

pkg/kn/commands/route/describe.go Outdated Show resolved Hide resolved

rhuss reviewed Feb 10, 2020

View reviewed changes

navidshaikh added 2 commits February 10, 2020 16:44

Print Service heading irrespective of owner.Kind == Service

50adaf7

Align desc, err msg for trigger, apiserver and cronjob source

81e3fdc

knative-prow-robot assigned rhuss Feb 10, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2020

knative-prow-robot merged commit 15dec58 into knative:master Feb 10, 2020

navidshaikh deleted the pr/describe-route branch February 10, 2020 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement human readable output for kn route describe #643

Implement human readable output for kn route describe #643

navidshaikh commented Feb 7, 2020 •

edited

Loading

knative-prow-robot left a comment

navidshaikh commented Feb 7, 2020

knative-prow-robot left a comment

rhuss commented Feb 7, 2020

rhuss commented Feb 7, 2020

rhuss commented Feb 7, 2020

rhuss left a comment

rhuss commented Feb 7, 2020

maximilien left a comment •

edited

Loading

knative-prow-robot commented Feb 7, 2020

rhuss commented Feb 10, 2020

navidshaikh commented Feb 10, 2020

rhuss left a comment

rhuss commented Feb 10, 2020

knative-metrics-robot commented Feb 10, 2020

navidshaikh commented Feb 10, 2020

rhuss commented Feb 10, 2020

rhuss commented Feb 10, 2020

Implement human readable output for kn route describe #643

Implement human readable output for kn route describe #643

Conversation

navidshaikh commented Feb 7, 2020 • edited Loading

Proposed Changes

knative-prow-robot left a comment

Choose a reason for hiding this comment

Proposed Changes

navidshaikh commented Feb 7, 2020

knative-prow-robot left a comment

Choose a reason for hiding this comment

rhuss commented Feb 7, 2020

rhuss commented Feb 7, 2020

rhuss commented Feb 7, 2020

rhuss left a comment

Choose a reason for hiding this comment

rhuss commented Feb 7, 2020

maximilien left a comment • edited Loading

Choose a reason for hiding this comment

knative-prow-robot commented Feb 7, 2020

rhuss commented Feb 10, 2020

navidshaikh commented Feb 10, 2020

rhuss left a comment

Choose a reason for hiding this comment

rhuss commented Feb 10, 2020

knative-metrics-robot commented Feb 10, 2020

navidshaikh commented Feb 10, 2020

rhuss commented Feb 10, 2020

rhuss commented Feb 10, 2020

navidshaikh commented Feb 7, 2020 •

edited

Loading

maximilien left a comment •

edited

Loading