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

Print the revision name after service create and update commands #290

Closed
navidshaikh opened this issue Jul 24, 2019 · 11 comments
Closed

Print the revision name after service create and update commands #290

navidshaikh opened this issue Jul 24, 2019 · 11 comments
Assignees
Labels
kind/feature New feature or request

Comments

@navidshaikh
Copy link
Collaborator

navidshaikh commented Jul 24, 2019

/kind feature

Describe the feature:

When a user updates a service, we print the success message

➜ kn service update bluegreen --env TARGET=green
Waiting for service 'bluegreen' to become ready ... OK
Service 'bluegreen' updated in namespace 'default'.

however, we should also print the revision name generated after this update, which can be used to refer that revision for further operations, eg: traffic splitting. This can also be used for scripting use cases.

Update:
Implement same behavior for service create command as well.

@navidshaikh navidshaikh added the kind/feature New feature or request label Jul 24, 2019
@duglin
Copy link
Contributor

duglin commented Jul 24, 2019

they should also be allowed to specify the new traffic split on the update cmd

@navidshaikh
Copy link
Collaborator Author

they should also be allowed to specify the new traffic split on the update cmd

sorry I didn't get that, could you please add some more detail ?

@navidshaikh navidshaikh changed the title Print the revision name generated after service update command Print the revision name after service create and update commands Jul 24, 2019
@duglin
Copy link
Contributor

duglin commented Jul 24, 2019

Sorry I really should have put that comment on the other issue - see: #285 (comment)

@rhuss
Copy link
Contributor

rhuss commented Jul 25, 2019

Implement same behavior for service create command as well.

For kn service create we print the URL at the end. The service name does not make sense, as its provided as argument.

With having the URL on a single line as last argument you can do things like

curl $(kn service create hello --image ... | tail -1)

Not sure if this a good thing when people will rely on that or if a kn service describe hello --url is better suited (I tend to the latter)

@navidshaikh
Copy link
Collaborator Author

Before printing the URL, we can add a line may be saying

Created revision 'svc-abcde'
[..]

keeping the URL line at the bottom only.
WDYT ?

@duglin
Copy link
Contributor

duglin commented Jul 25, 2019

I agree with @rhuss on the tail thing - I tend to use stuff like that in bash-scripts all the time.

@navidshaikh
Copy link
Collaborator Author

@duglin @rhuss : the tail use case is intact, I am saying to have a line in output (not the last line) printing revision name.. like

✗  kn service create hello --image docker.io/swordphilic/go-helloworld
Service 'hello' successfully created in namespace 'default'.
Revision 'hello-asdf' created..
Waiting for service 'hello' to become ready ... OK

Service URL:
http://hello.default.apps-crc.testing
✗  kn service update hello --env TARGET=v2                            
Revision hello-wxyz created ...
Waiting for service 'hello' to become ready ... OK
Service 'hello' updated in namespace 'default'.

WDYT ?

@duglin
Copy link
Contributor

duglin commented Aug 21, 2019

@navidshaikh looks good - easily parse-able by scripts :-)

w.r.t. the tail thing... I'm ok with it "as is", but if people are concerned about other things possibly showing up in the output after the URL, then we could make it:

Service URL: http://hello.default.apps-crc.testing

Probably safer for grep-ing. Just a thought.

@navidshaikh
Copy link
Collaborator Author

/assign

@navidshaikh
Copy link
Collaborator Author

/unassign @navidshaikh
/assign @rhuss

@rhuss
Copy link
Contributor

rhuss commented Oct 30, 2019

This is now already implemented with #431

@rhuss rhuss closed this as completed Oct 30, 2019
coryrc pushed a commit to coryrc/client that referenced this issue May 14, 2020
If the test finishes before the signal is received, the test fails. Sleeping for 999s avoids that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants