Refactor common runner code to not use 3rd party dependencies#240
Conversation
| import javax.annotation.Nullable; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.apache.commons.lang3.StringUtils; |
There was a problem hiding this comment.
You should also remove/exclude the dependency from the pom, otherwise over time this will happen again
velo
left a comment
velo
left a comment
There was a problem hiding this comment.
I don't think this solves the underline issue.
Flink uses commons-lang3:3:12:0
We must nudge anyone building UDFs to use the same set of dependencies as flink-sql-runner uses.
|
I proposed the following alternative: A single mandatory dependency to all UDFs, we can even let sqrl to automatically manage it to match compiler version (then we need to build this dependency on sqrl side of the fence) |
|
My only goal here was to make sure the common runner logic cannot break because the user uses some different dependency versions that are included in Flink, and that is achieved with this. The Flink dep version does not matter, cause the SQL runner code will be executed as "user code" in Flink, so it will utilize the UserClassLoader, which will load deps from the passed JAR (flink-sql-runner-uber) and any UDF JARs if those are provided. I agree that populating the proper runner dependency versions is a more sophisticated thing to do, but that will also require SQRL-side changes, and should not be shipped in a patch version. And making |
2 participants
Since by default Flink uses child-first class-loading, if a user ships a wrong version of a 3rd party dep we use in the common deployment code, it may break before deploy, so we should only use Flink-specific deps and picocli in any
flink-sql-runnermodule class.