alexandre-normand commented on PR #1749:
URL: https://github.com/apache/iceberg-go/pull/1749#issuecomment-5255173259
> Thanks — implementation is clean and the test is solid. Two things before
this can move forward:
>
> 1. +1 to the open question from `alessandro-nori` on per-request vs.
catalog-level configuration. Java configures `snapshot-loading-mode` once at
catalog initialization, and this repo followed a similar context/config pattern
for `pageSize`. I'd like to hear your take before we commit to the
`LoadTableWithSnapshotMode` public API shape — a catalog property (possibly
alongside this method) may serve the common case better.
It's probably too late to chime in but here's another alternative to having
a new public API or passing things through the context (which is not ideal for
discovery of the feature).
The idea is just functional options (more go idiomatic and easier for
discovery) and this is kind of a high-level of what this could look like. Note
that existing callers would just have the same existing/default behavior and
users that need to set a different snapshot mode would just need
```go
type LoadTableOption func(c *loadTableCall)
type loadTableCall struct {
snapshotMode SnapshotMode
}
func WithLoadTableSnapshotMode(m SnapshotMode) LoadTableOption {
return func(c *loadTableCall) {
c.snapshotMode = m
}
}
func (r *Catalog) LoadTable(ctx context.Context, identifier
table.Identifier, opts ...LoadTableOption) (*table.Table, error) {
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
