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

Keep yessql.db as the fallback name for sqlite for backward compatability #15154

Closed
MikeAlhayek opened this issue Jan 23, 2024 · 9 comments
Closed
Labels

Comments

@MikeAlhayek
Copy link
Member

@hishamco PR #7446 introduced a breaking change. All of my local tenants that use Sqlite are failing because they are expecting a file called yessql.db and not the app is looking for OrchardCore.db.

Here is what we should do. When creating new sites that use Sqllite, save "DatabaseName": "OrchardCore.db", to the shellSettings. If the shell-settings does not have DatabaseName value, then use yessql.db. This way we keep it backward compatible and only change it for new tenants. Ping me when you get this fix

@hishamco
Copy link
Member

hishamco commented Jan 23, 2024

I mentioned that in the breaking changes section, hope this https://github.com/OrchardCMS/OrchardCore/blob/main/src/docs/releases/1.9.0.md#data-access should fix the issue

@hishamco
Copy link
Member

Here is what we should do. When creating new sites that use Sqllite, save "DatabaseName": "OrchardCore.db", to the shellSettings. If the shell-settings does not have DatabaseName value, then use yessql.db. This way we keep it backward compatible and only change it for new tenants. Ping me when you get this fix

Let me check ..

@hishamco
Copy link
Member

Here is what we should do. When creating new sites that use Sqllite, save "DatabaseName": "OrchardCore.db", to the shellSettings.

This is already done

@MikeAlhayek
Copy link
Member Author

@hishamco I don't think so. Where do you store that value when a new tenant is setup? This should be done in the SetupService. You'll need somethis like

shellSettings["DatabaseName"] = context.Properties.TryGetValue(SetupConstants.DatabaseName, out var dbName) ? dbName?.ToString() : null;

on line 162.

Look where SetupConstants.DatabaseTablePrefix is used, and you'll likely need to add SetupConstants.DatabaseName there too.

I think you'll need to remove the default value from SqliteOptions DatabaseName then in the SqliteHelper you can check if the app settings contains DatabaseName and use it otherwise use yessql

@hishamco
Copy link
Member

I don't think so.

You can create a new tenant :)

@MikeAlhayek
Copy link
Member Author

@hishamco I just did as you can see the databasename is not written to the Shellsettings of the tenants.

image

@hishamco
Copy link
Member

Ya, even before the my PR, please refer to the related PR

@MikeAlhayek
Copy link
Member Author

correct. So what I am suggestion is that for new tenants, you shoud write entry in that file "DatabaseName": "orchardcore.db". If that property exists, then we use the name listed in the settings, otherwise, we use yessql.db.

@MikeAlhayek MikeAlhayek changed the title Keep yessql.db as the fallback name for sqllite for backward compatability Keep yessql.db as the fallback name for sqlite for backward compatability Jan 23, 2024
@Piedone
Copy link
Member

Piedone commented Jan 28, 2024

The original change was reverted in ea5ada3 so this is not applicable anymore.

@Piedone Piedone closed this as not planned Won't fix, can't repro, duplicate, stale Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants