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

feat: Support persisting SQLite DB, and data refresher #233

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

lukehesluke
Copy link
Contributor

@lukehesluke lukehesluke commented Feb 26, 2025

This PR builds on top of the changes from this PR: #156. The main difference is this:

  • It is less ambitious. Rather than supporting SQL Server, for external persistent storage, it just uses SQLite. The storage persists by simply re-using the same file

I did not quite have time to set up CI tests for this (though this has been partially complete). So, that has been put into another (wip) PR: #237

Possible goals for a later date, which are not being pursued here (they would need sufficient usecase (e.g. use at scale) to be worth pursuing in a future issue):

  • Support SQL Server (or PostgreSQL / MySQL / etc). This would be more reliable and scalable
  • Support SQL migrations, so that schema changes can be made without needing to clear an existing database

QA

Using the CI script from #237:

➜  OpenActive.Server.NET git:(feature/persistent-db-ci) ✗ node scripts/testPersistentDatabase.js 
Starting test process...
1a). Please spawn a RefImpl process in the background with the specified env vars. Type "y" when done: y
Continuing with RefImpl running...
(node:55518) Warning: Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to '0' makes TLS connections and HTTPS requests insecure by disabling certificate verification.
(Use `node --trace-warnings ...` to show where the warning was created)
Put old opportunity into ref impl db: 200 {
  '@context': 'https://openactive.io/',
  '@type': 'ScheduledSession',
  '@id': 'https://localhost:5001/api/identifiers/scheduled-sessions/1/events/1'
}
1c). Please spawn a Broker process in the background with the specified env vars. Type "y" when done: y
Continuing with Broker running...
Awaiting Broker health check...
Broker health check passed
✅ Confirmed that past-bookable opportunity was found in broker feed
✅ Asserted that there is no bookable opportunity in broker feed
Please kill the Broker and RefImpl processes. Type "y" when done: y
Continuing after killing processes...
2a). Please spawn a RefImpl process with data refresher enabled. Type "y" when done: y
Continuing with RefImpl and data refresher running...
Data refresher has completed its first cycle
2c). Please spawn a Broker process (same as in step 1c). Type "y" when done: y
Continuing with Broker running...
Awaiting Broker health check...
Broker health check passed
✅ Asserted that there is a bookable opportunity in broker feed
2e). Please kill all processes and prepare to exit. Type "y" when done: y
Test completed successfully!
➜  OpenActive.Server.NET git:(feature/persistent-db-ci) ✗ 

@@ -336,3 +336,6 @@ ASALocalRun/

# Fake database
*fakedatabase.db

# Output path for app publishing
/web-app-package/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this had accidentally entered the git repo at some point

@@ -0,0 +1,70 @@
using System;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is very similar to FakeDataRefresherService from #156, except with the following changes:

  • Logs
  • It informs DataRefresherStatusService when a has completed a cycle (which is explained in that file)

typeof(SellerUserTable)
};

private static readonly Type[] TableTypesToDrop = new []
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private static readonly Type[] TableTypesToDrop = new []
// This is TableTypesToCreate in reverse order
private static readonly Type[] TableTypesToDrop = new []

Copy link
Contributor

@civsiv civsiv Mar 13, 2025

Choose a reason for hiding this comment

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

I spent 30s wondering what was different between the two despite the comment below as I saw this first

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