tanmayrauth commented on PR #1430: URL: https://github.com/apache/iceberg-go/pull/1430#issuecomment-5004977089
@zeroshade the comment was misleading, and you're right that reading from r.props didn't actually avoid server-vended selection. r.props is toProps(ops) after fetchConfig merges the /v1/config defaults and overrides, so a server-vended metrics-reporter-impl was already folded in — an unregistered name (e.g. a Java class) would fail the client's load/create/register even when the client never configured metrics. Fixed by keeping a separate client-supplied set: init now captures reporterProps from the client options before fetchConfig merges server config, and reporter resolution (load, create, register) reads reporterProps instead of r.props. So a server-vended impl — registered or not — can no longer select or break the client's reporter; with no client selection, ops fall back to nop. Added TestServerVendedReporterImplIgnored (server vends does-not-exist via overrides, client sets nothing → create succeeds with a nop reporter), which fails against the old r.props path. -- 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]
