Finalize only if db is available

[lekoala]

Fixes #77

Description

See issue for context

Manual testing steps

Issues

#77

Pull request checklist

• The target branch is correct
  • See picking the right version
• All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
• The commit messages follow our commit message guidelines
• The PR follows our contribution guidelines
• Code changes follow our coding conventions
• This change is covered with tests (or tests aren't necessary for this change)
• Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• CI is green

[lekoala]

pr failure not related
see no such function: FIELD => seems that the eager loading feature using sqlite incompatible syntax

[GuySartorelli]

One small suggested change, though I don't use sqlite so really it's up to you either way.

Would you like to be added as a maintainer for this module, since it's no longer commercially maintained?

[GuySartorelli]

Would isActive() be more appropriate to call here?

[lekoala]

yes is active should work just as well but brings no additional benefit except being probably more correct semantically

https://github.com/silverstripe/silverstripe-sqlite3/blob/1e8d7357be321b3eb32a596538078fd65d0d357e/code/SQLite3Connector.php#L184C21-L188

if you see, unloading the db remove the databaseName and close the connection. isActive only checks if there is a dbConn object... and there would be a closed one. :-) so either way should work just fine. i don't think there is a way to check if the dbConn was indeed closed.

[GuySartorelli]

Yup that's what I was thinking too. If you change it to isActive() I'll be happy to merge. I like semantically correct ;p

[lekoala]

@GuySartorelli i'm happy to be added as a maintainer but i know i'm obviously less strict than you guys are when it comes to releasing new versions ;-)
as long as you are comfortable if I merge things I see necessary in my projects then I'm happy to share what I find useful with the community
but i don't plan to add extra burden on my already busy schedule :-)

[GuySartorelli]

i know i'm obviously less strict than you guys are when it comes to releasing new versions ;-)

That's fine - this is no longer a commercially supported module, so it's not restricted by the minor or major release policies.

as long as you are comfortable if I merge things I see necessary in my projects then I'm happy to share what I find useful with the community

Yup, no worries there. It's effectively a community module at this stage - just no community member has stepped up to maintain it yet. I'll add you as a maintainer.

Edit: Ignore the email that invites you to a team - I thought I could create a repo-level team but that doesn't seem to be the case. I've added you as a maintainer directly to this repository.

[GuySartorelli]

LGTM, as you said, the ci failures are unrelated.