return execution id on pipeline execute command #264
Conversation
|
The following commits need their title changed:
Please format your commit title into the form:
This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here. |
There was a problem hiding this comment.
hey @suhrud-kumar you're right that we need to have the fix in gate.
I'm hesitant to merge this because it's going to get dropped in the next release because we just replace the autogenerated code blindly.
yes, I agree. I just updated code here so that it could serve as pointer for what needs to be fixed in gate(which i'm not familiar with). Do you want me to open an issue with gate for this ? |
|
Yeah, you can open a issue in spinnaker/spinnaker to update it and someone else might take it. |
|
I don't think this is fixed in the gate yet but I think my PR could help here too: #311 we often need to get the execution IDs for pipelines for various reasons too. |
|
Any update on this? Looks like Spin CLI still does not support this. It's a crucial feature. |
|
could we update this PR with latest master branch? 🙏 Just being able to see the response returned I think would be helpful and enables users to parse it as needed. |
|
Any updates on this? Its a cruical thing to have. |
| @@ -87,7 +87,7 @@ func executePipeline(cmd *cobra.Command, options *executeOptions) error { | |||
| } | |||
| } | |||
|
|
|||
| resp, err := options.GateClient.PipelineControllerApi.InvokePipelineConfigUsingPOST1(options.GateClient.Context, | |||
| successPayload, resp, err := options.GateClient.PipelineControllerApi.InvokePipelineConfigUsingPOST1(options.GateClient.Context, | |||
There was a problem hiding this comment.
Instead of doing the JSON decoding in the generated file, why not attempt a JSON decode of the response body resp here?
There was a problem hiding this comment.
It looks like it might be better to use the newer Gate API options.GateClient.PipelineControllerApi.InvokePipelineConfigViaEchoUsingPOST instead.
This returns an interface{} which we can marshal into a new struct, e.g: type executedPipelines struct {...} and then log out.
The struct will hold a list of executed pipelines because a single trigger (spin's POST) can invoke multiple pipelines.
See Gate groovy returned object which would land in the Go empty interface.
return [
eventId: eventId,
ref : String.format("/pipelines/%s", executionId)
]
| if err = json.NewDecoder(localVarHttpResponse.Body).Decode(&successPayload); err != nil { | ||
| return successPayload, localVarHttpResponse, err | ||
| } |
There was a problem hiding this comment.
I think this can be moved to execute.go per my comment above.
Then do a pass to ensure errors are checked and resp.Body is closed, etc.
|
shall we merge this PR ? |
We are building a wrapper around spin cli, so if spin cli could execution id then we could use it to poll spinnaker to get the status of the pipeline. Looks like the gate api already returns the pipeline execution id but we are not returning it to user. I made a couple of changes for this to work.
before code changes
after
I understand the code in gateapi is auto-generated using swagger, so probably something needs to be updated in gate service as well (probably here - https://github.com/spinnaker/gate/blob/master/gate-web/src/main/groovy/com/netflix/spinnaker/gate/controllers/PipelineController.groovy#L198). I'm not well versed with java so I'm not sure what needs to be changed there. I'm updating my code here, so that we know what needs to be changed.
This also fixes - spinnaker/spinnaker#5657
cc: @kevinawoo