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

File Persistence and MongoDB persistence #178

Merged
merged 6 commits into from
Mar 25, 2025
Merged

File Persistence and MongoDB persistence #178

merged 6 commits into from
Mar 25, 2025

Conversation

Croway
Copy link
Contributor

@Croway Croway commented Mar 17, 2025

Persistence core module, at the moment 2 persistence flavour are implemented:

  • File, the default one, is enabled if there are no other persistence-related beans in the context. This flavor works like the actual persistence on file system
  • MongoDB, uses quarkus-mongodb-client, and is enabled if the property wanaku.persistence.mongodb is set to true

@Croway Croway requested review from orpiske and ibek March 17, 2025 11:13
Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, it looks good to me and I think you are on the right track.

There's a few improvements I think we could have on the code:

  • try to add javadoc to the core persistence api
  • I'd prefer if we named it core-persistence-file, core-persistence-api and core-persistence-mongodb: the reason being that as we add more and more components for the code, it starts to get a bit confusing to navigate in the project tree.
  • Make sure to add the MongoDB container (or Postgres) in the docker-compose.yml file.
  • Also make sure to add documentation about this on the usage guide.

Thanks!

@Croway Croway force-pushed the core-persistence branch 4 times, most recently from 0fa438d to db71d70 Compare March 18, 2025 13:55
Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see all the improvements @Croway! There are still some things to tweak, but it's getting better and better. I'm looking forward seeing this one on the code 🙏

Comment on lines 33 to 36

#quarkus.mongodb.connection-string = mongodb://localhost:27017
#wanaku.persistence.mongodb = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is out of scope, as downstream services should not interact w/ the persistence layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct

@orpiske
Copy link
Contributor

orpiske commented Mar 24, 2025

Per discussion, this should also resolve #190.

@orpiske orpiske marked this pull request as ready for review March 24, 2025 15:17
Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ready to go, thanks!

Croway added 4 commits March 24, 2025 18:53
rename persistence modules

Use repository in wanaku server

File Persistence and MongoDB persistence

Use repository in wanaku server

rebase main

fix Resources test
@Croway Croway force-pushed the core-persistence branch from 5a41eee to f8a7be5 Compare March 24, 2025 17:54
@Croway Croway force-pushed the core-persistence branch from f8a7be5 to 3fdf159 Compare March 25, 2025 11:00
@Croway Croway merged commit 1b31dec into main Mar 25, 2025
2 checks passed
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.

Consolidate service configuration Improve indexes persistency
3 participants