Add sql-runner module and deployment example #2
Conversation
Signed-off-by: Gantigmaa Selenge <[email protected]>
There was a problem hiding this comment.
Thanks @tinaselenge
Did upstream have any unit tests we can extend? It would be great to get some coverage on both classes.
flink-sql-runner/src/main/java/com/github/streamshub/flink/KubernetesSecretReplacer.java
Outdated
Show resolved
Hide resolved
flink-sql-runner/src/main/java/com/github/streamshub/flink/KubernetesSecretReplacer.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks for making a start on this @tinaselenge @katheris I don't have anything that needs fixed in in this PR
| String namespace = matcher.group(1); | ||
| String secretName = matcher.group(2); | ||
| String secretKey = matcher.group(3); | ||
| String secretValue = getSecretValue(namespace, secretName, secretKey); |
There was a problem hiding this comment.
I think we should actually extract the resolver as an interface and inject that into this class (happy to do/see that work done separately) but it would make testing this a lot easier.
There was a problem hiding this comment.
I have noted this in the issue for adding unit tests.
flink-sql-runner/src/main/java/com/github/streamshub/flink/KubernetesSecretReplacer.java
Show resolved
Hide resolved
flink-sql-runner/src/main/java/com/github/streamshub/flink/KubernetesSecretReplacer.java
Outdated
Show resolved
Hide resolved
flink-sql-runner/src/main/java/com/github/streamshub/flink/SqlRunner.java
Outdated
Show resolved
Hide resolved
flink-sql-runner/src/main/java/com/github/streamshub/flink/SqlRunner.java
Outdated
Show resolved
Hide resolved
|
Thank you @robobario @SamBarker for reviewing the PR. I have addressed the comments and raised issues for the suggested improvements to make later. |
No description provided.