pulsar-flink icon indicating copy to clipboard operation
pulsar-flink copied to clipboard

Published 20 hours ago • verified publisher icon streamnative

#386: Implemented passing of `CryptoKeyReader`.

Open objecttrouve opened this issue 3 years ago • 6 comments

Implemented passing a CryptoKeyReader (and encryption keys) to FlinkPulsarSource and FlinkPulsarSink, as requested here.

Used builder pattern for easy extensibility without breaking or excessively overloading public c'tors.

Added test. (Maybe it can be moved to one of the other tests to avoid overhead.)

objecttrouve avatar Aug 08 '21 13:08 objecttrouve

@jianyun8023 @syhily could you help take a look?

nlu90 avatar Aug 10 '21 18:08 nlu90

Hi @objecttrouve, tks for your contribution. Using a builder pattern on internal Fetcher don't sounds like a Good design. Add builder pattern for public source & sink is accepted, by we may need extra validation on final build method.

The detailed review advice would be given in weekend.

syhily avatar Aug 27 '21 15:08 syhily

@syhily, thanks for the feedback!

I'll make the requested changes in my next free time slot.

objecttrouve avatar Aug 28 '21 10:08 objecttrouve

@syhily, I made the changes as I understood your requests. Please let me know if there's anything else. Also, if I should squash. (Left multiple commits for better relatability to your remarks. For the time being.)

objecttrouve avatar Sep 15 '21 15:09 objecttrouve

Thanks for your contribution. The PR looks reasonable on my side. I will merge this on the weekend.

syhily avatar Sep 15 '21 15:09 syhily

Thanks for your contribution. The PR looks reasonable on my side. I will merge this on the weekend.

Thanks @syhily!

objecttrouve avatar Sep 21 '21 18:09 objecttrouve