-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for object attributes in Iter call #63
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,11 +6,13 @@ package objstore | |
import ( | ||
"bytes" | ||
"context" | ||
"fmt" | ||
"io" | ||
"io/fs" | ||
"os" | ||
"path" | ||
"path/filepath" | ||
"slices" | ||
"strings" | ||
"sync" | ||
"time" | ||
|
@@ -70,8 +72,19 @@ type InstrumentedBucket interface { | |
type BucketReader interface { | ||
// Iter calls f for each entry in the given directory (not recursive.). The argument to f is the full | ||
// object name including the prefix of the inspected directory. | ||
|
||
// Entries are passed to function in sorted order. | ||
Iter(ctx context.Context, dir string, f func(string) error, options ...IterOption) error | ||
Iter(ctx context.Context, dir string, f func(name string) error, options ...IterOption) error | ||
|
||
// IterWithAttributes calls f for each entry in the given directory similar to Iter. | ||
// In addition to Name, it also includes requested object attributes in the argument to f. | ||
// | ||
// Attributes can be requested using IterOption. | ||
// Not all IterOptions are supported by all providers, requesting for an unsupported option will fail with ErrOptionNotSupported. | ||
IterWithAttributes(ctx context.Context, dir string, f func(attrs IterObjectAttributes) error, options ...IterOption) error | ||
|
||
// SupportedIterOptions returns a list of supported IterOptions by the underlying provider. | ||
SupportedIterOptions() []IterOptionType | ||
|
||
// Get returns a reader for the given object name. | ||
Get(ctx context.Context, name string) (io.ReadCloser, error) | ||
|
@@ -101,24 +114,66 @@ type InstrumentedBucketReader interface { | |
ReaderWithExpectedErrs(IsOpFailureExpectedFunc) BucketReader | ||
} | ||
|
||
var ErrOptionNotSupported = errors.New("iter option is not supported") | ||
|
||
// IterOptionType is used for type-safe option support checking. | ||
type IterOptionType int | ||
|
||
const ( | ||
Recursive IterOptionType = iota | ||
UpdatedAt | ||
) | ||
|
||
// IterOption configures the provided params. | ||
type IterOption func(params *IterParams) | ||
type IterOption struct { | ||
Type IterOptionType | ||
Apply func(params *IterParams) | ||
} | ||
|
||
// WithRecursiveIter is an option that can be applied to Iter() to recursively list objects | ||
// in the bucket. | ||
func WithRecursiveIter(params *IterParams) { | ||
params.Recursive = true | ||
func WithRecursiveIter() IterOption { | ||
return IterOption{ | ||
Type: Recursive, | ||
Apply: func(params *IterParams) { | ||
params.Recursive = true | ||
}, | ||
} | ||
} | ||
|
||
// WithUpdatedAt is an option that can be applied to Iter() to | ||
// include the last modified time in the attributes. | ||
// NB: Prefixes may not report last modified time. | ||
// This option is currently supported for the azure, aws, bos, gcs and filesystem providers. | ||
func WithUpdatedAt() IterOption { | ||
return IterOption{ | ||
Type: UpdatedAt, | ||
Apply: func(params *IterParams) { | ||
params.LastModified = true | ||
}, | ||
} | ||
} | ||
|
||
// IterParams holds the Iter() parameters and is used by objstore clients implementations. | ||
type IterParams struct { | ||
Recursive bool | ||
Recursive bool | ||
LastModified bool | ||
} | ||
|
||
func ValidateIterOptions(supportedOptions []IterOptionType, options ...IterOption) error { | ||
for _, opt := range options { | ||
if !slices.Contains(supportedOptions, opt.Type) { | ||
return fmt.Errorf("%w: %v", ErrOptionNotSupported, opt.Type) | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func ApplyIterOptions(options ...IterOption) IterParams { | ||
out := IterParams{} | ||
for _, opt := range options { | ||
opt(&out) | ||
opt.Apply(&out) | ||
} | ||
return out | ||
} | ||
|
@@ -189,6 +244,20 @@ type ObjectAttributes struct { | |
LastModified time.Time `json:"last_modified"` | ||
} | ||
|
||
type IterObjectAttributes struct { | ||
Name string | ||
lastModified time.Time | ||
} | ||
|
||
func (i *IterObjectAttributes) SetLastModified(t time.Time) { | ||
i.lastModified = t | ||
} | ||
Comment on lines
+252
to
+254
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious question: why did we make assignments to lastModified come through a setter but not name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i wanted to make it explicit that |
||
|
||
// LastModified returns the timestamp the object was last modified. Returns false if the timestamp is not available. | ||
func (i *IterObjectAttributes) LastModified() (time.Time, bool) { | ||
return i.lastModified, !i.lastModified.IsZero() | ||
} | ||
|
||
// TryToGetSize tries to get upfront size from reader. | ||
// Some implementations may return only size of unread data in the reader, so it's best to call this method before | ||
// doing any reading. | ||
|
@@ -531,7 +600,7 @@ func (b *metricBucket) ReaderWithExpectedErrs(fn IsOpFailureExpectedFunc) Bucket | |
return b.WithExpectedErrs(fn) | ||
} | ||
|
||
func (b *metricBucket) Iter(ctx context.Context, dir string, f func(name string) error, options ...IterOption) error { | ||
func (b *metricBucket) Iter(ctx context.Context, dir string, f func(string) error, options ...IterOption) error { | ||
const op = OpIter | ||
b.metrics.ops.WithLabelValues(op).Inc() | ||
|
||
|
@@ -546,6 +615,23 @@ func (b *metricBucket) Iter(ctx context.Context, dir string, f func(name string) | |
return err | ||
} | ||
|
||
func (b *metricBucket) IterWithAttributes(ctx context.Context, dir string, f func(IterObjectAttributes) error, options ...IterOption) error { | ||
const op = OpIter | ||
b.metrics.ops.WithLabelValues(op).Inc() | ||
|
||
err := b.bkt.IterWithAttributes(ctx, dir, f, options...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updates to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be there now. PTAL again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM! |
||
if err != nil { | ||
if !b.metrics.isOpFailureExpected(err) && ctx.Err() != context.Canceled { | ||
b.metrics.opsFailures.WithLabelValues(op).Inc() | ||
} | ||
} | ||
return err | ||
} | ||
|
||
func (b *metricBucket) SupportedIterOptions() []IterOptionType { | ||
return b.bkt.SupportedIterOptions() | ||
} | ||
|
||
func (b *metricBucket) Attributes(ctx context.Context, name string) (ObjectAttributes, error) { | ||
const op = OpAttributes | ||
b.metrics.ops.WithLabelValues(op).Inc() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: just to match the provider name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks.