tanmayrauth commented on code in PR #1330:
URL: https://github.com/apache/iceberg-go/pull/1330#discussion_r3486542225
##########
catalog/rest/rest.go:
##########
@@ -451,6 +451,8 @@ func handleNon200(rsp *http.Response, override
map[int]error) error {
func fromProps(props iceberg.Properties, o *options) {
for k, v := range props {
switch k {
+ case keyOauthToken:
+ o.oauthToken = v
Review Comment:
Nice, this is the actual fix: before, `token` fell through to the `default`
case and landed in `additionalProps`, so `setupOAuthManager` (which reads
`opts.oauthToken`) never saw it and property-driven auth was silently a no-op.
Wiring it into the typed field also means the very first `/v1/config` call is
now authenticated.
##########
catalog/rest/rest.go:
##########
@@ -508,6 +510,7 @@ func toProps(o *options) iceberg.Properties {
}
}
+ setIf(keyOauthToken, o.oauthToken)
Review Comment:
Good that you added this alongside the fromProps change — it's required now
that `token` no longer lands in `additionalProps` (which toProps copies via
maps.Copy), so without this the token would drop out of the catalog's retained
props. Sits right next to the existing `credential` setIf, so it's consistent.
##########
catalog/rest/rest_internal_test.go:
##########
@@ -1081,6 +1092,18 @@ func TestToPropsSigv4RegionFallback(t *testing.T) {
})
}
+func TestFromPropsReadsOAuthToken(t *testing.T) {
+ t.Parallel()
+
+ var opts options
+ fromProps(iceberg.Properties{
+ keyOauthToken: "static-token",
+ }, &opts)
+
+ assert.Equal(t, "static-token", opts.oauthToken)
Review Comment:
The `assert.Nil(t, opts.additionalProps)` here is the assertion I'd most
want, it proves the token isn't double-stored in the free-form bucket.
--
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]
