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.

Sign up for GitHub

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

Jump to bottom

service get #48

Closed
csantanapr opened this issue Apr 2, 2019 · 14 comments
Closed

service get #48

csantanapr opened this issue Apr 2, 2019 · 14 comments
Milestone
v0.1.0

Comments

@csantanapr
Copy link
Member

implement kn service get <service_name>
@csantanapr
Copy link
Member Author

/milestone v0.1.0

@knative-prow-robot
Copy link
Contributor

@csantanapr: You must be a member of the knative/knative-milestone-maintainers github team to set the milestone.

In response to this:

/milestone v0.1.0

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.

@csantanapr csantanapr added this to the v0.1.0 milestone Apr 5, 2019
@navidshaikh
Copy link
Collaborator

@csantanapr : what information about <service_name> should be printed in this command ?

How does this defer from kn service list?

If we can list the data points to be provided by CLI for these two commands, it should help.

@rhuss
Copy link
Contributor

rhuss commented Apr 12, 2019

I would like to suggest to combine kn service list and kn service get to a single command kn service get for the following reasons:

  • It reduces the UX surface by having one subcommand less. This is IMO always a good thing. kn service get without argument can just list all services.
  • It mimics the behaviour of kubectl get where kubectl get without argument just list all resources. So having only kn service get also follows the Principle of least astonishment which is a good thing.

Wdyt ?

If there are no objections, I'd like to start working on kn service get

@rhuss
Copy link
Contributor

rhuss commented Apr 12, 2019

Another argument for having a single 'kn service get' for listing and showing is a typical ux flow:

  • List all service available
  • Then: Select a single service to get details.

So you can call 'kn service get', check the output, cursor up for one step back in shell history and then just add the service of interest to the end of the command.

@rhuss rhuss mentioned this issue Apr 12, 2019
Closed
@cppforlife
Copy link

@rhuss downside to combining list and get is that i imagine output format would be quite different. list i imagine is more compact whereas get would show more detailed information.

@sixolet
Copy link
Contributor

sixolet commented Apr 15, 2019

I like having different list and single-object describe commands. Get vs describe, no particluarly strong opinion about the single-object one, but I do like describe.

@rhuss
Copy link
Contributor

rhuss commented Apr 16, 2019

@sixolet +1, I think too that we should distinguish between get and describe a la kubectl.

I think we could just rename kn service list to kn service get and allow an argument for filtering on the list. That would be similar to kubectl get (and hence less surprising).

kn service describe would be reserved then for a detail view of a single service.

Wdyt ?

Of course it depends how close we want to follow a UX like for kubectl. I think it would be good to stay close to its UX (get for list/summary like info, describe for details) because people know this already. On the other hand when changing to something new, then I would do it for all verbs. I.e. change also describe to something else, like show or info(so, list for list/summary and show for details, new for create).

@rhuss
Copy link
Contributor

rhuss commented Apr 16, 2019

@rhuss downside to combining list and get is that i imagine output format would be quite different. list i imagine is more compact whereas get would show more detailed information.

@cppforlife I'm not sure whether the output format should be different, if we want to keep the semantics of get in kubectl (which actually is a list command there).

Operation kubectl style kn style alternate style
Summary info get list list
Details info describe describe / (get now ?) show

rhuss added a commit to rhuss/knative-client that referenced this issue Apr 17, 2019
@rhuss
feature(service describe): Output of service details
b61c3eb 
As discussed in knative#48 this `kn service describe` as changed in this commit
mimics `kubectl describe <sth>` as that it output detail information
in human readable output.

Summary information of a list of services or a single services as well
as exporting the service in various formats (JSON, YAML) is reserved to
`kn service list` (or `kn service get` as suggested to align with
kubectl conventions).

For this reason, the generic Printer handling from cli-runtime has been
removed as `kn service describe` only outputs human readable format
(that is btw also true for `kubectl` describe which does not allow for
`-o` kind of options).

The presented information is just a start and the full content
should be discussed.

I could imagine to add the following additional fields in the output:

- Revisions overview (like image)
- Traffic distribution
- Timeout seconds
- Conditions
- Relevant Events (if any)
- Annotations / Labels
- Reference to configuration

Surely there can be more, but I suggest to make this an iterative excercise.
@rhuss rhuss mentioned this issue Apr 17, 2019
Closed
@rhuss
Copy link
Contributor

rhuss commented Apr 17, 2019

I suggest #75 for the start of implementation for this issue with kn service describe is about presenting the detail information (instead of kn service get).

@sixolet
Copy link
Contributor

sixolet commented Apr 18, 2019

I think one thing I don't like about kubectl is the fact that get works on two different cardnalities.

Operation Cardnality kubectl current kn
Summary One get n/a
Summary Many get list
Detail One describe describe
Detail Many n/a n/a

@sixolet
Copy link
Contributor

sixolet commented Apr 18, 2019

(I'm ok with implementing a single-target summary information printing command, but it should be different from the many-target summary-information printing command)

@rhuss
Copy link
Contributor

rhuss commented Apr 18, 2019

(I'm ok with implementing a single-target summary information printing command, but it should be different from the many-target summary-information printing command)

For get (or list right now) we are supporting the printing option -o from cli-runtime, which treats single entities and a list of entities the same as it e.g. for yaml/json exports either a single object or a list of objects, but with the same content. Same for -o name.

In that sense and for staying consistent, I tend to lean to keep the one and many information the same also for 'human readable' output for kn get (i.e. using a list with a single entry).

What would be a plus though would be to allow multiple and wildcard arguments for kn service get to filter the list (see #71 )

@maximilien maximilien mentioned this issue May 10, 2019
Merged
@sixolet
Copy link
Contributor

sixolet commented May 14, 2019

Done by #90

@sixolet sixolet closed this as completed May 14, 2019
rhuss added a commit to rhuss/knative-client that referenced this issue May 15, 2019
@rhuss
feature(service describe): Output of service details
fe8c894 
As discussed in knative#48 this `kn service describe` as changed in this commit
mimics `kubectl describe <sth>` as that it output detail information
in human readable output.

Summary information of a list of services or a single services as well
as exporting the service in various formats (JSON, YAML) is reserved to
`kn service list` (or `kn service get` as suggested to align with
kubectl conventions).

For this reason, the generic Printer handling from cli-runtime has been
removed as `kn service describe` only outputs human readable format
(that is btw also true for `kubectl` describe which does not allow for
`-o` kind of options).

The presented information is just a start and the full content
should be discussed.

I could imagine to add the following additional fields in the output:

- Revisions overview (like image)
- Traffic distribution
- Timeout seconds
- Conditions
- Relevant Events (if any)
- Annotations / Labels
- Reference to configuration

Surely there can be more, but I suggest to make this an iterative excercise.
@rhuss rhuss mentioned this issue May 19, 2019
Closed
rhuss added a commit to rhuss/knative-client that referenced this issue May 28, 2019
@rhuss
feature(service describe): Output of service details
a44413c 
As discussed in knative#48 this `kn service describe` as changed in this commit
mimics `kubectl describe <sth>` as that it output detail information
in human readable output.

Summary information of a list of services or a single services as well
as exporting the service in various formats (JSON, YAML) is reserved to
`kn service get`

For this reason, the generic Printer handling from cli-runtime has been
removed as `kn service describe` only outputs human readable format
(that is btw also true for `kubectl` describe which does not allow for
`-o` kind of options).

This command shows information about the serivce itself, but also a summary about the associated revisions.

"kn service describe" knows three modes, which can be combined:

`--all`    : Print more information. By default only are shorter summary is shown
`--color`  : Use a colorful output (but only when on a tty)
`--hipster`: Experimental output mode for a very compact representation using emojis

Of course there can be more and other information to be shown. Let's discuss and add this (and maybe remove the dense format)
rhuss added a commit to rhuss/knative-client that referenced this issue May 30, 2019
@rhuss
feature(service describe): Output of service details
744872f 
As discussed in knative#48 this `kn service describe` as changed in this commit
mimics `kubectl describe <sth>` as that it output detail information
in human readable output.

Summary information of a list of services or a single services as well
as exporting the service in various formats (JSON, YAML) is reserved to
`kn service get`

For this reason, the generic Printer handling from cli-runtime has been
removed as `kn service describe` only outputs human readable format
(that is btw also true for `kubectl` describe which does not allow for
`-o` kind of options).

This command shows information about the serivce itself, but also a summary about the associated revisions.

"kn service describe" knows three modes, which can be combined:

`--all`    : Print more information. By default only are shorter summary is shown
`--color`  : Use a colorful output (but only when on a tty)
`--hipster`: Experimental output mode for a very compact representation using emojis

Of course there can be more and other information to be shown. Let's discuss and add this (and maybe remove the dense format)
rhuss added a commit to rhuss/knative-client that referenced this issue Jul 8, 2019
@rhuss
feature(service describe): Output of service details
ac4b85a 
As discussed in knative#48 this `kn service describe` as changed in this commit
mimics `kubectl describe <sth>` as that it output detail information
in human readable output.

Summary information of a list of services or a single services as well
as exporting the service in various formats (JSON, YAML) is reserved to
`kn service get`

For this reason, the generic Printer handling from cli-runtime has been
removed as `kn service describe` only outputs human readable format
(that is btw also true for `kubectl` describe which does not allow for
`-o` kind of options).

This command shows information about the serivce itself, but also a summary about the associated revisions.

"kn service describe" knows three modes, which can be combined:

`--all`    : Print more information. By default only are shorter summary is shown
`--color`  : Use a colorful output (but only when on a tty)
`--hipster`: Experimental output mode for a very compact representation using emojis

Of course there can be more and other information to be shown. Let's discuss and add this (and maybe remove the dense format)
coryrc pushed a commit to coryrc/client that referenced this issue May 14, 2020
@adrcunha @knative-prow-robot
Make the teardown() function optional for e2e-tests.sh (knative#48)
bd4527a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.1.0
Development

No branches or pull requests
6 participants
@rhuss @cppforlife @csantanapr @sixolet @navidshaikh @knative-prow-robot