delta-io/connectors

Passing options to delta flink sink

gopik opened this issue · 4 comments

gopik commented

Hi,
I would recommend to do it via .option(...). And the implementation for .option(...) for Delta Sink Builder in my opinion should be implemented in separate branch, reviewed and merged before this one. @scottsand-db sounds good to you?

The hadoop conf is needed/used only by delta standalone. There is no interaction between Flink specific classes from Delta connector with hadoop conf and I think we should need to have a good reason to change this.

@gopik
you can check DeltaSourceBuilderBase.java to see how .option(...) was implemented there and do the same thing for DeltaSinkBuilder. Heads up, both builder implementations differ in low level details.

Also please note that .option(...) is overloaded, it accepts boolean, int, long and String as option values. Yours needs to have the same.

As for the feature flag, you need to have a DeltaSourceOptions.java like class for Sink and add the flag there as a DeltaConfigOption instance.

A good example to follow would be IGNORE_DELETES option from DeltaSourceOptions.java. Having the API for .option(...) you can use the same definition as IGNORE_DELETES has, use same BooleanOptionTypeConverter and it should all be working.

Originally posted by @kristoffSC in #393 (comment)

gopik commented

@kristoffSC I've created this issue to track options work as discussed in #393 .

Do we want to replicate the structure (DeltaSourceConfiguration, DeltaSourceOptions, DeltaConfigOption etc) within sink? Or should we extract common structure in common? DeltaSourceConfiguration and DeltaConfigOption seem generic and DeltaSourceOptions seems to be specific to source.

@gopik
thanks for creating this issue, adding the description with our previous discussion and thank you for starting development for it.

Regarding your questions, I believe that:

  1. DeltaSourceConfiguration -> can be used for both Sink and Source. You would have to change its name to something like DeltaConnectorConfiguration and move to common package but still under "internal".

  2. DeltaSourceOptions is Source specific and you would have to create analogous class for Sink, for example DeltaSinkOptions

  3. DeltaConfigOption can/should be reused by both, Sink and Source. In this case you just need to move it to common package but keep it under "internal".

gopik commented

Hey @kristoffSC please take a look at the approach in the PR - #425

This PR is incomplete, only shared for quick feedback.