itsbilal commented on code in PR #2075:
URL: https://github.com/apache/iceberg-go/pull/2075#discussion_r4167095823
##########
catalog/rest/rest.go:
##########
@@ -1302,6 +1303,7 @@ func (r *Catalog) tableFromResponse(
r,
table.WithMetricsReporter(reporter),
table.WithScanPlanningIOProperties(scanPlanningConfig),
+ table.WithSavedConfig(config),
Review Comment:
No worries at all - thanks so much for the detailed response! Totally
understand how the PR volume can be overwhelming (+ the pace of development on
this project almost requires AI use). I just wanted an actual opinion and not a
bot's here haha.
The layering makes sense now that we think of it as "table-specific but not
catalog-internal props". I've clarified this in the table.WithSavedConfig
comment now, and taken away the confusing example of storing credentials there.
I've also implemented your suggestion as-is, really appreciated!
--
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]
