danielcweeks commented on pull request #1767: URL: https://github.com/apache/iceberg/pull/1767#issuecomment-730503042
> > > > @jackye1995 I've moved this PR to draft for now. I rebased on top of your changes in #1754, but it's somewhat complicated due to the fact that now we have 3 separate requests (create multipart, upload part, put object) that all require setting the properties, but they don't inherit from a common interface. I feel like reflection is probably the best way do simplify this, but I'll push the current state of things. > > > > > > > > > Sorry I was dealing with some other errands yesterday. Please see #1786 to see if that solves this issue, I am trying to not use reflection, but we can also switch to that approach if it is simpler. > > > > > > Turns out I did something very similar here: https://github.com/apache/iceberg/pull/1767/files#diff-133c36e9cbb025f7cb44c2daac330c501d85a5a6eb44126c0eb6155f2bad7407R30 > > Oh yes, looks like It is a generalization for that class. For the name, I would prefer the class to be more generic just to avoid creating other utils for future use cases. For reflection, I am not sure what is the community guideline here but personally I would avoid using that, which resulted in that solution using functional interface. @jackye1995 I actually stepped back from using reflection as I found out that some request types do not set all of the same parameters (e.g. UploadPartRequest, GetObjectRequest aren't exactly the same as PutObjectRequest and CreateMultipartUploadRequest). Since there are no common interfaces, it seems reasonable to just separate out the behavior for now. As we get into other instances of this (like ACL, etc.), then maybe it would make sense to find a more concise solution. I've got a few small changes to make and want to expand the testing, but hopefully have something ready for review today. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
