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

Change Cache to Embedded H2 #113

Merged
merged 34 commits into from
May 30, 2022
Merged

Change Cache to Embedded H2 #113

merged 34 commits into from
May 30, 2022

Conversation

Attacktive
Copy link
Contributor

It resolves #110.

As a Kotlin noob, I am not sure if it's good enough but my two cents here.

I chose H2 only because I'm kind of familiar with it.
Switching to another like Nitrite or Derby shouldn't be a big deal.

You can enable the console by making a few changes.

I decided to have a single table with a JSON column which contains every bit of information in one out of laziness. 😒

@Attacktive
Copy link
Contributor Author

Thank you so much, @iProdigy.
I'll be working on it soon.

Copy link
Contributor

@iProdigy iProdigy left a comment

Choose a reason for hiding this comment

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

Nice work! Code looks good to me - will download H2 & test this out

@tipsy requesting your review :)

@Attacktive
Copy link
Contributor Author

Thank you sooo much, @iProdigy for helping me learn to code in Kotlin.

@iProdigy
Copy link
Contributor

Thank you sooo much, @iProdigy for helping me learn to code in Kotlin.

my pleasure! thanks for bearing with my nits

@tipsy
Copy link
Owner

tipsy commented May 25, 2022

Thanks @iProdigy and @Attacktive, I will have a look in the weekend !

Comment on lines 30 to 32
val connection = HikariCpDataSource.connection

createTableIfAbsent()
Copy link
Owner

Choose a reason for hiding this comment

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

Switch these two? The createTableIfAbsent() method creates its own connection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, missed this one. It's also resolved by the below.

@tipsy
Copy link
Owner

tipsy commented May 28, 2022

@Attacktive there seems to be a lot of changes here that are unrelated to the caching (particularly in UserService), is that correct?

@Attacktive
Copy link
Contributor Author

@Attacktive there seems to be a lot of changes here that are unrelated to the caching (particularly in UserService), is that correct?

Ah, the changes in UserService might look excessive, but actually they mostly are:

That's about it. 😁

@tipsy
Copy link
Owner

tipsy commented May 30, 2022

That's about it. 😁

Perfect! Nice work @Attacktive, thank you very much ☺️

@tipsy tipsy merged commit f7488ae into tipsy:master May 30, 2022
@Attacktive Attacktive deleted the h2-cache branch May 31, 2022 00:46
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.

Change Cache to be an embedded DB
3 participants