Skip to content
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

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

mackerl
Copy link
Contributor

@mackerl mackerl commented Nov 23, 2023

  • introduce CrateDbDialect ** specializing on differences to Postgres 14
    ** 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

Summary of the changes / Why this is an improvement

Checklist

  • Link to issue this PR refers to (if applicable):
  • CLA is signed

@matriv matriv self-requested a review November 23, 2023 15:19
Copy link
Contributor

@matriv matriv left a 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).

@mackerl
Copy link
Contributor Author

mackerl commented Mar 7, 2024

Hello,

tried to address the review items including a new junit test with testcontainers.

BR Mario

Copy link
Contributor

@matriv matriv left a 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.

@mackerl
Copy link
Contributor Author

mackerl commented May 8, 2024

done if I got everything correctly :)

Copy link
Contributor

@matriv matriv left a 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.

@matriv
Copy link
Contributor

matriv commented May 17, 2024

@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

@matriv
Copy link
Contributor

matriv commented May 17, 2024

Additionally when attempting to run the Application I get:

ERROR: The LogManager accessed before the "java.util.logging.manager" system property was set to "org.jboss.logmanager.LogManager". Results may be unexpected.

@mackerl
Copy link
Contributor Author

mackerl commented May 25, 2024 via email

@mackerl
Copy link
Contributor Author

mackerl commented Jun 5, 2024

@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.
quarkusdev worked for me when specialising a correct database in application.properties - as I saw other test susing localhost:5432/testdrive I changed to that DB - unless there is a crateDB in quarkus devcontainers I see no way to create it on the fly. (https://quarkus.io/guides/databases-dev-services )

** 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
Copy link
Contributor

@matriv matriv left a 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!

@matriv matriv merged commit a00262e into crate:main Jun 5, 2024
@mackerl mackerl deleted the quarkus-example branch June 25, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants