search

RedPillAnalytics / gradle-confluent

Apache License 2.0
22 stars 10 forks source link

optional statement-pause between create statements #89

Closed terryf82 closed 4 years ago

terryf82 commented 4 years ago

Allow for a configurable pause between execution of create statements, for pipelines that execute 'too quickly' for full ksql joining.

stewartbryson commented 4 years ago

Thanks a bunch for the contribution @terryf82. Is there a reason you used a String for the datatype of the pause value?

stewartbryson commented 4 years ago

The pause parameter is a good one @terryf82. Before merging, I just want to see if perhaps it makes more sense as an int. Let me know your thinking here, and let's get this merged. Thank you!

terryf82 commented 4 years ago

@stewartbryson I had originally tried an integer, but was getting an unexpected exception:

org.gradle.api.internal.tasks.options.OptionValidationException: Option 'statement-pause' cannot be cast to type 'java.lang.Integer' in class 'com.redpillanalytics.gradle.tasks.PipelineExecuteTask'.

I've updated the P.R. to reflect the code that is causing this exception, if you have any ideas where I've gone wrong please let me know. Thanks.

stewartbryson commented 4 years ago

@stewartbryson I had originally tried an integer, but was getting an unexpected exception:

org.gradle.api.internal.tasks.options.OptionValidationException: Option 'statement-pause' cannot be cast to type 'java.lang.Integer' in class 'com.redpillanalytics.gradle.tasks.PipelineExecuteTask'.

I've updated the P.R. to reflect the code that is causing this exception, if you have any ideas where I've gone wrong please let me know. Thanks.

Options don't support integers... my bad. Go ahead and put it back to String. Sorry for the hassle. Also... the build is failing because I upgraded the docker image to 5.5.1, and some of the SQL in the test case is no longer compliant. Working on that.

stewartbryson commented 4 years ago

@terryf82 If you merge master back into your branch, and push it, along with your changes to put pause back to a String, I think we should see a successful build.

terryf82 commented 4 years ago

@stewartbryson thanks, I should have picked up on Integer not being an allowed input type too. Changes have been reverted and I've merged in master, let me know if anything further.