Destination CDK: Simplify AsyncStreamConsumer constructors #37106
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
gisripa
changed the title
| * @return PartialAirbyteMessage if the message is valid, empty otherwise | ||
| */ | ||
| fun deserializeAirbyteMessage( | ||
| messageString: String?, | ||
| dataTransformer: StreamAwareDataTransformer, |
There was a problem hiding this comment.
love how the doc string didn't even match the parameters...
| @@ -144,7 +146,7 @@ private constructor( | |||
| writeConfigs, | |||
| typerDeduper | |||
| ), // todo (cgardens) - wrapping the old close function to avoid more code churn. | |||
| OnCloseFunction { _, streamSyncSummaries -> | |||
| { _, streamSyncSummaries -> | |||
There was a problem hiding this comment.
now that we're in kotlin (no checked exceptions)... can we kill try-catch here? (... or just use the function returned from GeneralStagingFunctions.onCloseFunction directly)
feels weird to build a lambda that effectively just wraps another lambda
There was a problem hiding this comment.
yeah let me see if i can unify. i have no clue what the java equivalent code was 😅
There was a problem hiding this comment.
Simplified it and yet another cgardens TODO gone 😄
There was a problem hiding this comment.
we should track the number of times the string cgardens shows up in our code as a vanity metric :P
gisripa
force-pushed
the
gireesh/04-12-destinations-cdk-simplify-ctors
branch
from
358ab5b
to
bf9a6e4
Compare
gisripa
force-pushed
the
gireesh/04-12-destinations-cdk-simplify-ctors
branch
from
bf9a6e4
to
9d92625
Compare
|
/publish-java-cdk
|
TL;DR
This pull request removes complex constructors and unifies into single constructor in
AsyncStreamConsumerand related classes.What changed?
DeserializationUtiltoAirbyteMessageDeserializerTransformer&DeserializerAsyncStreamConsumerStagingConsumerFactory&JdbcBufferedConsumerFactory