dramaticlly commented on PR #17638: URL: https://github.com/apache/iceberg/pull/17638#issuecomment-5350417806
> >Note withCredentials appends rather than replaces, so toBuilder().withCredentials(extra) preserves any existing credentials; the TestRESTScanPlanning simplification is behavior-preserving. > > > > This is actually quite surprising imo. I know it's an existing behavior but I feel like that's definitley not expected from a builder API; I forget if this was intentional or not, maybe @singhpk234 remembers. I guess it's not an issue hit in practice but I feel like we should have this replace credentials, and if we have a need for adding, we could do an `addCredentials`. Thanks @amogh-jahagirdar , once we figured out the expected behaviour, happy to follow up and give a pass for all builder method deal with collections. -- 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]
