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

Deprecate FileStore engine support #2119

Merged
merged 8 commits into from
Jun 13, 2024
Merged

Deprecate FileStore engine support #2119

merged 8 commits into from
Jun 13, 2024

Conversation

bcmmbaga
Copy link
Contributor

@bcmmbaga bcmmbaga commented Jun 11, 2024

Describe your changes

Deprecates support for JSON file storage.

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

bcmmbaga added 4 commits June 10, 2024 18:01
The codebase has been refactored to remove support for JSON file store storage engine, with SQLite serving as the default store engine. New functions have been added to handle unsupported store engines and to migrate data from file store to SQLite.
@bcmmbaga bcmmbaga force-pushed the remove-filestore branch 2 times, most recently from b4d88cc to 940aac2 Compare June 11, 2024 16:05
@bcmmbaga bcmmbaga marked this pull request as ready for review June 12, 2024 06:46
@bcmmbaga bcmmbaga force-pushed the remove-filestore branch 2 times, most recently from af06be4 to 0143afe Compare June 12, 2024 08:11
if err := checkFileStoreEngine(kind, dataDir); err != nil {
return nil, err
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not clear the case handleUnsupportedStoreEngine and the checkFileStoreEngine
The handleUnsupportedStoreEngine tries to migrate the json file to SQL. But before that, the checkFileStoreEngine will return with error because the JSON file exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checkFileStoreEngine only checks if store.json exists and jsonfile is set as the store engine in management.json. It then returns an error with a link pointing to the steps for migrating from jsonfile to SQLite.

handleUnsupportedStoreEngine handles the case when the store engine is not supported. If there is an existing store.json, it automatically migrates to SQLite, but the user will need to update the store engine to SQLite. If there is no existing store.json, it returns an error indicating an unsupported store engine

// if store engine is not set in the config we first try to evaluate NETBIRD_STORE_ENGINE
kind := getStoreEngineFromEnv()
if kind == "" {
// NETBIRD_STORE_ENGINE is not set we evaluate default based on dataDir
kind = getStoreEngineFromDatadir(dataDir)
kind = SqliteStoreEngine
Copy link
Contributor

@pappz pappz Jun 12, 2024

Choose a reason for hiding this comment

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

The comment is not valid any more above this line

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@bcmmbaga bcmmbaga merged commit 95299be into main Jun 13, 2024
24 checks passed
@bcmmbaga bcmmbaga deleted the remove-filestore branch June 13, 2024 10:39
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