-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat:Support Bytes to UUID UDF Function #9640
Conversation
883499b
to
a7d731f
Compare
Hi. It does not work well during the build process. Please check below. Link CI Error
|
@jaceklaskowski @jnh5y @JimGalasyn @cprasad1 @nateab @pgaref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks for the contribution! Once you get a green build on CI feel free to merge
ksqldb-engine/src/main/java/io/confluent/ksql/function/udf/string/Uuid.java
Outdated
Show resolved
Hide resolved
hmm the CI failure might be an issue with our build process, i will raise it to our tools team |
@pkgonan while we wait for tools to help us resolve, i ran your branch locally but i ran into issues with checkstyle, i would reccomend running checkstyle, spotbugs, and the tests locally as well. You can use something like |
@nateab |
@nateab Upon closer inspection, it appears to be a problem with a specific test within ksqldb-engine. However, this seems to be a problem unrelated to this UUID feature PR. I checked out the master branch and found the same error as a result of running the corresponding test codes. CI Test Result
ksqldb-engine Test
|
hey @pkgonan looking at the latest ci run https://jenkins.public.confluent.io/job/ksql/job/PR-9640/10/execution/node/33/log/, i see this failure now instead
|
@nateab I think that CI is not stable and broken. The version of the io.confluent.common project in pom.xml committed to the master branch is 7.4.0-430 ( Line 32 in 3f96077
CI Error
https://jenkins.public.confluent.io/job/ksql/job/PR-9640/12/console |
81b3f8b
to
9fd9521
Compare
@nateab It seems that the master branch is not stable. It looks like a non-existent snapshot version is being committed to master branch. The source code hasn't changed, but the results of the tests keep changing. |
@pkgonan Yes there are some currently known issues around our build system right now, but choosing master as the base branch is still correct. I will follow up with tools again to try to resolve the build issue. In the meanwhile does your PR pass locally at least? If so, I can also just build your PR locally to verify and then merge if we cant sort out the build issues before then |
@nateab To build in your local environment, I have to download the dependencies in the Confluent Snapshot Repository. So of course a local build cannot succeed. But the test for the UUID feature I added works fine. |
@pkgonan okay ive verified that it passes all checks locally so ill go ahead and merge it, thanks for your first contribution to ksqlDB! 🥳 |
@pkgonan i think if you add |
@pkgonan hey it would be good to also add some documentation on this new UDF here please in a follow up PR https://docs.ksqldb.io/en/latest/developer-guide/ksqldb-reference/scalar-functions/ |
@nateab Hi. Thank you very much. I'll add the docs via a new PR. |
@nateab Finally, i added docs. Would you please check that? Docs PR |
Issue