-
Notifications
You must be signed in to change notification settings - Fork 7
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
Adding a working example with Crate DB and panache #160
Conversation
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.
Thank you so much for this effort, the PR is in a very good state, I left some mostly minor comments. Apart from those, I'd kindly ask you to include some more data types to the test Entity, and to please enrich the app with more actions, like updating, deleting, quering, etc.
Is it possible to also have some tests that assert the functionality? (apart from the app itself).
by-language/java-quarkus-panache/src/main/java/io/crate/MyEntity.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/MyEntity.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/MyApplication.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/CrateDbDialect.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/CrateDbDialect.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/MyEntity.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/CrateDbDialect.java
Outdated
Show resolved
Hide resolved
by-language/java-quarkus-panache/src/main/java/io/crate/CrateDbDialect.java
Outdated
Show resolved
Hide resolved
Hello, tried to address the review items including a new junit test with testcontainers. BR Mario |
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.
Thank you @mackerl!
Just left a couple of last comments. Could you please address them and squash your commits and then I can merge.
by-language/java-quarkus-panache/src/test/java/io/crate/test/CrateDbTest.java
Outdated
Show resolved
Hide resolved
done if I got everything correctly :) |
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.
Thx so much for the contribution @mackerl !
Will just give a final check locally and merge soon.
@mackerl, Unfortunately running I get an error:
|
Additionally when attempting to run the Application I get:
|
I will have a look
Am Fr., 17. Mai 2024 um 15:57 Uhr schrieb Marios Trivyzas <
***@***.***>:
… @mackerl <https://github.com/mackerl>, Unfortunately running ./gradlew
clean test doesn't work, nor ./gradlew quarkusDev
I get an error:
Caused by: java.lang.NullPointerException: Cannot invoke "com.fasterxml.jackson.databind.ObjectMapper.constructType(java.lang.reflect.Type)" because "this.objectMapper" is null
—
Reply to this email directly, view it on GitHub
<#160 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZEWUA24ERVJVF7XUONBDTZCYEC5AVCNFSM6AAAAAA7XSP76OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJXGY3DSMBVGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Dipl.-Ing. Mario Ackerl
Softwareentwickler
***@***.***
|
@matriv I made a fix to the test - it seems a little bit weird, that Quarkus optimized the objectmapper out, but I prevented that now. |
** no sequences ** different timestamp creation (no prescision) ** map JSON as OBJECT(DYNAMIC) ** map array as array(elementTypeName) instead of ... elementTypeName array ** exclude unsupported features introduce CrateJsonType without casting (postgres default) show it on an example panache entity
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.
Thx so much @mackerl !
I'm merging the PR!
** no sequences
** different timestamp creation (no prescision)
** map JSON as OBJECT(DYNAMIC)
** map array as array(elementTypeName) instead of ... elementTypeName array ** exclude unsupported features
Summary of the changes / Why this is an improvement
Checklist