Return pipeline run bundle from UI_GetPipelineRun

[jgwhite]

Why the change?

This was an oversight on my part. We need the extra info for the UI.

What’s the plan?

• Update protos
• Update tests
• Fix tests

What does it look like?

Given I have run this pipeline.

When I run:

$ ./contrib/waypoint-grpc/waypoint-grpc.sh UI_GetPipelineRun '
{
  "pipeline": {
    "owner": {
      "project": {
        "project": "pipelines"
      },
      "pipeline_name": "one-step"
    }
  },
  "sequence": 1
}
'

Then I see this:

{
  "pipelineRunBundle": {
    "pipelineRun": {
      "id": "01H2AQSAABY33HNR8MS32V6JW5",
      "sequence": "1",
      "pipeline": {
        "id": "01H2AQRY5WCMXTP17DP7R9DPE5"
      },
      "jobs": [
        {
          "id": "01H2AQSAACRZGX35JGP5BSJD4V"
        }
      ],
      "state": "SUCCESS"
    },
    "queueTime": "2023-06-07T10:52:35.021352377Z",
    "application": {
      "application": "web",
      "project": "pipelines"
    },
    "dataSourceRef": {
      "git": {
        "commit": "a8925b436fadb7ad1f28750a0da8ff7175ba78e6",
        "timestamp": "2023-05-23T21:14:53Z",
        "commitMessage": "Add b\n"
      }
    }
  },
  "rootTreeNode": {
    // ...snip...
  }
}

How do I test it?

Essentially follow the steps above, in other words:

1. Run a pipeline
2. Use waypoint-grpc to call UI_GetPipelineRun
3. Verify pipelineRunBundle looks correct

[jgwhite]

Why no deprecated_ prefix?

Though changing the name of a field is backwards-compatible in end-to-end protobuf, it’s not backwards-compatible with Swagger and our generated client library.

[briancain]

What, really? That's surprising. That kind of breaks the one reason why protobufs are cool and useful for crafting APIs. It should be safe to rename fields in protobufs as long as the number stays the same. If we lose that, then that's a shame 😞

[briancain]

Also if we're deprecating this, then we're gonna need to update everywhere in the system that uses this field. I know much of the CLI is relying on this field existing.

[jgwhite]

What, really? That's surprising. That kind of breaks the one reason why protobufs are cool and useful for crafting APIs. It should be safe to rename fields in protobufs as long as the number stays the same. If we lose that, then that's a shame 😞

Absolutely. All is good when you stay within gRPC. But unfortunately because we go via Swagger and the OpenAPI generator, we lose that delightful property of protobufs (at least I’m pretty sure that’s the case). It affects the UI more than anything else.

Also if we're deprecating this, then we're gonna need to update everywhere in the system that uses this field. I know much of the CLI is relying on this field existing.

Oh, is the CLI using UI_GetPipelineRun?

[briancain]

Ohhh shoot, I think the git diff wasn't showing me the full picture, but isn't this modifying the GetPipelineRunResponse?

[briancain]

Ahhh, looks like this is within UI? So maybe not, all good I bet!

[jgwhite]

Yeah, should just be UI.GetPipelineRunResponse.

[andrew-hashicorp]

Looks good!