From cfa25489dd7d75ba759a5d5be9858f0ab5ce1a87 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Wed, 14 Aug 2024 10:00:35 +0000 Subject: [PATCH 1/3] set correct content types --- storage/gcp/gcp.go | 13 ++++++++----- storage/gcp/gcp_test.go | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/storage/gcp/gcp.go b/storage/gcp/gcp.go index fb8fd072..c6780037 100644 --- a/storage/gcp/gcp.go +++ b/storage/gcp/gcp.go @@ -56,6 +56,8 @@ import ( const ( entryBundleSize = 256 + appDataContType = "application/data" + txtContType = "text/plain; charset=utf-8" DefaultPushbackMaxOutstanding = 4096 DefaultIntegrationSizeLimit = 5 * 4096 @@ -79,7 +81,7 @@ type Storage struct { // objStore describes a type which can store and retrieve objects. type objStore interface { getObject(ctx context.Context, obj string) ([]byte, int64, error) - setObject(ctx context.Context, obj string, data []byte, cond *gcs.Conditions) error + setObject(ctx context.Context, obj string, data []byte, cond *gcs.Conditions, contType string) error } // sequencer describes a type which knows how to sequence entries. @@ -185,7 +187,7 @@ func (s *Storage) setTile(ctx context.Context, level, index, logSize uint64, til tPath := layout.TilePath(level, index, logSize) klog.V(2).Infof("StoreTile: %s (%d entries)", tPath, len(tile.Nodes)) - return s.objStore.setObject(ctx, tPath, data, &gcs.Conditions{DoesNotExist: true}) + return s.objStore.setObject(ctx, tPath, data, &gcs.Conditions{DoesNotExist: true}, appDataContType) } // getTiles returns the tiles with the given tile-coords for the specified log size. @@ -247,7 +249,7 @@ func (s *Storage) setEntryBundle(ctx context.Context, bundleIndex uint64, logSiz // Note that setObject does an idempotent interpretation of DoesNotExist - it only // returns an error if the named object exists _and_ contains different data to what's // passed in here. - if err := s.objStore.setObject(ctx, objName, bundleRaw, &gcs.Conditions{DoesNotExist: true}); err != nil { + if err := s.objStore.setObject(ctx, objName, bundleRaw, &gcs.Conditions{DoesNotExist: true}, appDataContType); err != nil { return fmt.Errorf("setObject(%q): %v", objName, err) } @@ -292,7 +294,7 @@ func (s *Storage) integrate(ctx context.Context, fromSeq uint64, entries []stora if err != nil { return fmt.Errorf("newCP: %v", err) } - if err := s.objStore.setObject(ctx, layout.CheckpointPath, cpRaw, nil); err != nil { + if err := s.objStore.setObject(ctx, layout.CheckpointPath, cpRaw, nil, txtContType); err != nil { switch ee := err.(type) { case *googleapi.Error: if ee.Code == http.StatusTooManyRequests { @@ -662,11 +664,12 @@ func (s *gcsStorage) getObject(ctx context.Context, obj string) ([]byte, int64, // Note that when preconditions are specified and are not met, an error will be returned *unless* // the currently stored data is bit-for-bit identical to the data to-be-written. // This is intended to provide idempotentency for writes. -func (s *gcsStorage) setObject(ctx context.Context, objName string, data []byte, cond *gcs.Conditions) error { +func (s *gcsStorage) setObject(ctx context.Context, objName string, data []byte, cond *gcs.Conditions, contType string) error { bkt := s.gcsClient.Bucket(s.bucket) obj := bkt.Object(objName) var w *gcs.Writer + w.ObjectAttrs.ContentType = contType if cond == nil { w = obj.NewWriter(ctx) diff --git a/storage/gcp/gcp_test.go b/storage/gcp/gcp_test.go index cfe79d90..70bbdfcc 100644 --- a/storage/gcp/gcp_test.go +++ b/storage/gcp/gcp_test.go @@ -325,7 +325,7 @@ func (m *memObjStore) getObject(_ context.Context, obj string) ([]byte, int64, e return d, 1, nil } -func (m *memObjStore) setObject(_ context.Context, obj string, data []byte, cond *gcs.Conditions) error { +func (m *memObjStore) setObject(_ context.Context, obj string, data []byte, cond *gcs.Conditions, _ string) error { m.Lock() defer m.Unlock() From 75763b4c4161af0d4b1abec1a4b2d6307b722328 Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Thu, 15 Aug 2024 07:32:29 +0000 Subject: [PATCH 2/3] swtich back to old octet-stream, and rename things --- storage/gcp/gcp.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/storage/gcp/gcp.go b/storage/gcp/gcp.go index c6780037..47797379 100644 --- a/storage/gcp/gcp.go +++ b/storage/gcp/gcp.go @@ -56,8 +56,8 @@ import ( const ( entryBundleSize = 256 - appDataContType = "application/data" - txtContType = "text/plain; charset=utf-8" + logContType = "application/octet-stream" + ckptContType = "text/plain; charset=utf-8" DefaultPushbackMaxOutstanding = 4096 DefaultIntegrationSizeLimit = 5 * 4096 @@ -187,7 +187,7 @@ func (s *Storage) setTile(ctx context.Context, level, index, logSize uint64, til tPath := layout.TilePath(level, index, logSize) klog.V(2).Infof("StoreTile: %s (%d entries)", tPath, len(tile.Nodes)) - return s.objStore.setObject(ctx, tPath, data, &gcs.Conditions{DoesNotExist: true}, appDataContType) + return s.objStore.setObject(ctx, tPath, data, &gcs.Conditions{DoesNotExist: true}, logContType) } // getTiles returns the tiles with the given tile-coords for the specified log size. @@ -249,7 +249,7 @@ func (s *Storage) setEntryBundle(ctx context.Context, bundleIndex uint64, logSiz // Note that setObject does an idempotent interpretation of DoesNotExist - it only // returns an error if the named object exists _and_ contains different data to what's // passed in here. - if err := s.objStore.setObject(ctx, objName, bundleRaw, &gcs.Conditions{DoesNotExist: true}, appDataContType); err != nil { + if err := s.objStore.setObject(ctx, objName, bundleRaw, &gcs.Conditions{DoesNotExist: true}, logContType); err != nil { return fmt.Errorf("setObject(%q): %v", objName, err) } @@ -294,7 +294,7 @@ func (s *Storage) integrate(ctx context.Context, fromSeq uint64, entries []stora if err != nil { return fmt.Errorf("newCP: %v", err) } - if err := s.objStore.setObject(ctx, layout.CheckpointPath, cpRaw, nil, txtContType); err != nil { + if err := s.objStore.setObject(ctx, layout.CheckpointPath, cpRaw, nil, ckptContType); err != nil { switch ee := err.(type) { case *googleapi.Error: if ee.Code == http.StatusTooManyRequests { From 14e62732cf8a137e8ed2052c07ad4edc751a8b2b Mon Sep 17 00:00:00 2001 From: Philippe Boneff Date: Thu, 15 Aug 2024 13:38:35 +0000 Subject: [PATCH 3/3] add todo for tests --- storage/gcp/gcp_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/gcp/gcp_test.go b/storage/gcp/gcp_test.go index 70bbdfcc..2fda4b72 100644 --- a/storage/gcp/gcp_test.go +++ b/storage/gcp/gcp_test.go @@ -325,6 +325,7 @@ func (m *memObjStore) getObject(_ context.Context, obj string) ([]byte, int64, e return d, 1, nil } +// TODO(phboneff): add content type tests func (m *memObjStore) setObject(_ context.Context, obj string, data []byte, cond *gcs.Conditions, _ string) error { m.Lock() defer m.Unlock()