[GitHub] [beam] jfarr commented on issue #9758: [BEAM-8374] Enable returning missing PublishResult fields in SnsIO.Write

[GitBox <gi...@apache.org>]

jfarr commented on issue #9758: [BEAM-8374] Enable returning missing PublishResult fields in SnsIO.Write
URL: https://github.com/apache/beam/pull/9758#issuecomment-596246326
 
 
   Hi @lukecwik and @iemejia, I believe I have incorporated all of your review feedback. I also made a few changes to the interface that I hope will improve clarity:
   
   - Instead of the confusingly named `fullSdkHttpMetadata()` and `minimalSdkHttpMetadata()` methods, the `AwsCoders` class now has a method `sdkHttpMetadata()` which creates a coder that by default does not include the response headers, and if you want to include the headers there is also a method `sdkHttpMetadata(boolean includeHeaders)`. I'm on the fence about getting rid of the boolean parameter and just including a method like `sdkHttpMetadataIncludingHeaders()` so let me know if you have a preference.
   
   - I changed the names of the SnsIO methods to `withFullPublishResultNoHeaders` and `withFullPublishResultIncludingHeaders`. The default is still to encode only the messageId for backward compatibility. I also added a method to specify your own coder for future extensibility.
   
   I hope this PR is ready to merge now, please let me know if it needs any additional changes.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services