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

cancel-build should take a buildconfig #5193

Closed
deads2k opened this issue Oct 18, 2015 · 15 comments
Closed

cancel-build should take a buildconfig #5193

deads2k opened this issue Oct 18, 2015 · 15 comments

Comments

@deads2k
Copy link
Contributor

deads2k commented Oct 18, 2015

If I call oc cancel-build with a buildconfig that has a single running build, cancel it for me.

@bparees
@Kargakis I think you're in this code at the moment.

@bparees
Copy link
Contributor

bparees commented Oct 18, 2015

No.

Maybe you ran it expecting to see usage information. Or hit enter
prematurely.

Cancelling is pretty destructive, I don't like the idea of defaulting it.

Ben Parees | OpenShift
On Oct 18, 2015 6:18 PM, "David Eads" notifications@github.com wrote:

If I call oc cancel-build with a buildconfig that has a single running
build, cancel it for me.

@bparees https://github.com/bparees
@Kargakis https://github.com/kargakis I think you're in this code at
the moment.


Reply to this email directly or view it on GitHub
#5193.

@deads2k
Copy link
Contributor Author

deads2k commented Oct 19, 2015

So I run oc start-build bc/foo, then say "crap, forgot to push my code", then try oc cancel-build bc/foo, and I should not get the behavior I expect? That seems kind of crazy.

Not that that ever happened to me. I'm asking for a friend...

@bparees
Copy link
Contributor

bparees commented Oct 19, 2015

  1. oc start-build reports back the name of the build it created (oddly without any context around the name). why wouldn't you use it?

  2. oc start-build bc/foo isn't the syntax to start a build from a buildconfig, oc start-build foo is.. so your cancel-build parallel invocation isn't going to be as parallel as you'd like.

@deads2k
Copy link
Contributor Author

deads2k commented Oct 19, 2015

The start-build input parsing bug will be fixed in #4947, so congruence can easily be achieved.

I didn't use the output of the oc start-build command because that came out 12 minute ago in my buffer and is long since gone.

I'm still not seeing why its a bad thing to have congruent commands that behave as expected. It would also handle the case of "I looked at oc status and I see one build running that I want to stop, but all I have is a buildconfig name". For deployment configs, we said that a DC is what users created and cared about, let them manipulate deployments using the object that expressed their intent as a resource. Are builds so different that the 90% case requires being familiar with the objects that I didn't create in addition to the one that I did?

@0xmichalis
Copy link
Contributor

It seems valid to support this. oc deploy config --cancel will cancel the latest deployment for the given config, I don't see why we shouldn't do the same for buildconfigs.

@liggitt
Copy link
Contributor

liggitt commented Oct 19, 2015

Maybe you ran it expecting to see usage information. Or hit enter prematurely.

The intent behind cancel-build bc/foo is pretty unambiguous... if that resolves to a single build, we should honor it, imo

@mfojtik
Copy link
Contributor

mfojtik commented Nov 10, 2015

@Kargakis should we then also support this behavior for logs? oc logs build-config-name should give me logs from the last build?

@0xmichalis
Copy link
Contributor

@Kargakis should we then also support this behavior for logs? oc logs build-config-name should give me logs from the last build?

oc logs bc/build-config-name already does that.

@mfojtik
Copy link
Contributor

mfojtik commented Nov 10, 2015

@Kargakis very cool :-)

@soltysh
Copy link
Contributor

soltysh commented Nov 10, 2015

It seems valid to support this. oc deploy config --cancel will cancel the latest deployment for the given config, I don't see why we shouldn't do the same for buildconfigs.

With deployments you can have only one deployment active, builds are different, you can have multiple build active and as @liggitt mentioned this command will be ambiguous in that case.

@deads2k
Copy link
Contributor Author

deads2k commented Nov 10, 2015

With deployments you can have only one deployment active, builds are different, you can have multiple build active and as @liggitt mentioned this command will be ambiguous in that case.

That's an uncommon case that can be inspected before completing the call. We can simply fail the command in that case.

@0xmichalis
Copy link
Contributor

That's an uncommon case that can be inspected before completing the call. We can simply fail the command in that case.

With something like "multiple builds are running in parallel".

@0xmichalis
Copy link
Contributor

Thinking about it, if I was trying to cancel a bc I would expect to cancel all of its builds... No?

@soltysh
Copy link
Contributor

soltysh commented Nov 10, 2015

Definitely not.

@0xmichalis
Copy link
Contributor

Fixed by #8509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants