Raise an explicit error when MySQL config is missing instead of a silent SQLite fallback

[jorisparet]

Addresses issue #3.

When DATABASE_URI is not provided and SQLITE=false, the settings code now requires all MySQL configuration values to be present instead of constructing an invalid URI with missing values. If any required MySQL setting is missing, a clear validation error is raised.

[asmleng]

Sorry about the failing checks, the tests were passing on my side.

I have identified the issue: tests/conftest.py indirectly imports settings, which calls settings = Settings(). Since by default, there is no .env or ~/.env.default file (unlike in my local copy of the repo), it leads to a Pydantic validation error because the Settings class defaults to SQLITE: bool = False.

I see two options:

1. We add .env.example as an option in env_file here:

model_config = SettingsConfigDict(
    case_sensitive=True,
    env_file=(str(Path.home() / ".env.default"), ".env.example", ".env"),
    extra="ignore",
)

but we can maybe print a warning to say that it's using the default file but that a .env should be created instead.

2. We do fallback to SQLite when there is no .env file available.

Do you have a preference?

[egrelier]

I think option 1 with a warning is best: it allows to pass the tests without a .env while warning the user to create one. Option 2 is too error-prone in my opinion.

What do you think?

[asmleng]

That's a good point. I agree, let's go with option 1 :)

I'll push these changes.

[jorisparet]

(Sorry, I'm mixing GitHub accounts 😅)

So, in the end it took me down an unexpected rabbit hole...

There is no (simple) way to check which environment file was used. Pydantic may use one file, or merge multiple files (i.e., you could have SQLITE=true defined in one file and SQLITE_DATABASE_FILE=... defined in another and it would still work).

Even worse, if environment variables have been manually defined (e.g. export SQLITE=true), they take precedence over any dotenv file and there is not way to know it. So (following what we agreed on), we could raise a warning if only .env.example is found, even though the user has actually set all the proper environment variables beforehand.

I tried to find the sweet spot between all these weird possible combinations. Essentially, if only .env.example is found, we raise a warning that says:

"Only the example dotenv file (.env.example) was found. If matching environment variables are set, they take precedence and .env.example may not affect the active configuration. To avoid ambiguity, create a real .env or ~/.env.default file, or remove .env.example." I think it covers all cases.

I pushed these changes. Unit tests now pass, but they raise a warning since there is only .env.example in the CI pipeline:

[...]
tests/test_models_reference.py::TestCategoryToDict::test_all_fields_present PASSED                                                 [100%]

============================================================ warnings summary ============================================================
event_storage/settings.py:42
  /home/joris/repositories/event-storage/event_storage/settings.py:42: RuntimeWarning: Only the example dotenv file (/home/joris/repositories/event-storage/.env.example) was found. If matching environment variables are set, they take precedence and .env.example may not affect the active configuration. To avoid ambiguity, create a real .env or ~/.env.default file, or remove .env.example.
    warnings.warn(

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
===================================================== 181 passed, 1 warning in 1.49s =====================================================

What do you think?

[egrelier]

Nice, seems perfect to me, thanks a lot! And good catch on removing the "thermal" in the example file 😅