feat(storage): add update time in bucketAttrs #10710
Conversation
product-auto-label
bot
added
the
api: storage
storage/bucket.go
Outdated
| @@ -416,6 +416,10 @@ type BucketAttrs struct { | |||
| // This field is read-only. | |||
| Created time.Time | |||
|
|
|||
| // Time at which the bucket was last modified. | |||
There was a problem hiding this comment.
Nit: godocs should start with the name of the field; ie Updated is the time at which the bucket was last modified. The linter will complain about this.
storage/bucket.go
Outdated
| @@ -922,6 +928,7 @@ func (b *BucketAttrs) toRawBucket() *raw.Bucket { | |||
| Name: b.Name, | |||
| Location: b.Location, | |||
| StorageClass: b.StorageClass, | |||
| Updated: b.Updated.Format(time.RFC3339), | |||
There was a problem hiding this comment.
Since the field is read-only, it doesn't have to be included in toRawBucket or toProtoBucket I believe.
storage/bucket_test.go
Outdated
| @@ -157,6 +158,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) { | |||
| }, | |||
| PublicAccessPrevention: "enforced", | |||
| }, | |||
| Updated: time.Now().Format("2006-01-02T15:04:05Z07:00"), | |||
There was a problem hiding this comment.
Does this actually work? Wouldn't time.Now() be later when the comparison is made?
There was a problem hiding this comment.
Given that this was BucketAttrs to RawBucket Test hence i have removed this.
Although a nice catch, but it was working somehow. Will check why.
storage/integration_test.go
Outdated
| @@ -589,6 +589,9 @@ func TestIntegration_BucketUpdate(t *testing.T) { | |||
| if !testutil.Equal(attrs.Labels, wantLabels) { | |||
| t.Fatalf("add labels: got %v, want %v", attrs.Labels, wantLabels) | |||
| } | |||
| if !attrs.Created.Before(attrs.Updated) { | |||
| t.Fatal("attrs.Updated should be newer than attrs.Created") | |||
There was a problem hiding this comment.
Nit: These checks should be Errorf rather than Fatalf because they don't need to stop the execution of the remainder of the test.
Also, include both timestamps in the error for debugging purposes, something like:
got attrs.Updated %v before attrs.Created %v, want Attrs.Updated to be after
There was a problem hiding this comment.
Have changed up.
Reason behind picking Fatalf was that Bucket Attributes should update given that this was not done hence i thought tests should stop. Also all other checks where fatalf only.
shubham-diwakar
force-pushed
the
updatetime
branch
from
01bdeb5 to
67b2c9e
Compare
kokoro-team
removed
the
kokoro:run
tritone
added
kokoro:run
gcf-merge-on-green
bot
removed
the
automerge
Add update time field in bucketAttrs struct.
Fixes #9361