feat: CBWP update with ETag #2012
Conversation
| @@ -88,6 +88,8 @@ type CloudBuildWorkerPoolObservedState struct { | |||
| // +required | |||
| WorkerConfig *WorkerConfig `json:"workerConfig,omitempty"` | |||
| NetworkConfig *NetworkConfigState `json:"networkConfig,omitempty"` | |||
|
|
|||
| ETag *string `json:"eTag,omitempty"` | |||
There was a problem hiding this comment.
It's still alpha, so not a blocker but ... I don't know how we should capitalize etag. I think it might just be etag. I don't think I've seen eTag anywhere else (?)
There was a problem hiding this comment.
Ah! You are right. It's interesting that the camel case format is etag with lower case e where the capitalized format is ETag.
Updated.
mockgcp/mockcloudbuild/workerpool.go
Outdated
| @@ -77,6 +77,7 @@ func (s *CloudBuildV1) CreateWorkerPool(ctx context.Context, req *pb.CreateWorke | |||
| result.CreateTime = now | |||
| result.UpdateTime = now | |||
| result.State = pb.WorkerPool_RUNNING | |||
| result.Etag = "W/11111111111111111111111111111111" | |||
There was a problem hiding this comment.
FYI I have some helper functions computeEtag that I typically put in utils.go in each mock directory. (And note that there are two versions, some which return strings, some which return []byte, because some services need string and some []byte)
Would be great to figure out a better place for them, but it's pretty easy to have more realistic etags :-)
There was a problem hiding this comment.
I like the computeEtag! Make some further changes (with weak indicator), please take a look at the latest commit. If the common computeEtag looks good, I can update the mockgcp
|
A few nits but lgtm. I'll hold in case you want to address them here. /hold |
yuwenma
force-pushed
the
cbwp-etag
branch
2 times, most recently
from
46641a1 to
0c2cdf3
Compare
|
/hold cancel |
|
Thanks @yuwenma /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-oss-prow
bot
merged commit 6944199
into
GoogleCloudPlatform:master
Add Etag to CloudBuildWorkerPool resource
status.ObservedState.When updating, we should pass in the etag to the CloudBuild server and expect a different etag from the response body.
Right now, the mockgcp replaces the ETag, so we can only see the diff from the golden object.