_init_archive_database silently falls back to a local SQLite file

[simontaurus]

Version

opensemantic.base 0.42.8.post1000002004006.

Summary

DataToolMixin._init_archive_database treats every failure to build the
PostgREST client as recoverable: it logs a warning and falls back to a
local SQLite file. A production tool then archives to disk instead of the
time series database, and nothing in the calling code can tell.

_controller_mixin.py:

except ImportError:
    _logger.warning("postgrest package not installed. Falling back to local SQLite.")
except Exception as e:
    _logger.warning("Could not create PostgREST controller for %s: %s."
                    " Falling back to local SQLite.", db.name, e)

# Fall back to local SQLite

The if server_url: guard has the same effect by omission: when no URL
can be derived, the PostgREST branch is skipped entirely and control
falls through to the SQLite path without even a warning.

Why it triggers in a normal setup

It does not require a misconfiguration. Given

Tool.storage_locations -> [Database]
Database.server        -> DatabaseServer

server is read at line 241 via db.__dict__.get("server") and then at
252 via getattr(db, "server"). Both return None, because a lazily
resolved entity comes back without its own __iris__
(OO-LD/oold-python#106). No URL is derivable, so the tool silently
archives to a local file.

Suggested fix

Make the fallback explicit rather than implicit. Something like an
allow_local_fallback flag defaulting to False, so a tool that
declares a remote storage location fails loudly when it cannot reach it,
and only an explicitly local setup writes to disk. Failing that, raise
when storage_locations names a remote database but no client could be
built.

Minor, same method

schema = getattr(server, "schema_", None) or "http"

The default contradicts the entity: our DatabaseServer declares
schema: https, and the attribute does read back as 'https' when the
server is resolved, so this is latent rather than active. But if the
attribute is ever unset the method will quietly build an http:// URL
for an https endpoint instead of reporting that the scheme is unknown.

Workaround in use

We build the PostgREST client ourselves from environment variables and
raise rather than degrade.

[simontaurus]

Proposed fix

Two independent changes: stop degrading silently, and stop guessing the scheme.

1. Make the local fallback opt-in

Add a controller-only flag and make every failure path raise unless it is set:

class DataToolMixin(BaseController):
    allow_local_archive_fallback: bool = False
    """Archive to a local SQLite file when the declared remote storage
    location cannot be reached. Off by default: a tool that declares a
    remote database should fail loudly rather than write to disk."""

In _init_archive_database:

if not server_url:
    if not self.allow_local_archive_fallback:
        raise ValueError(
            f"{db.name} declares no resolvable server, so no remote "
            "client can be built. Pass allow_local_archive_fallback=True "
            "to archive to a local file instead."
        )
    _logger.warning(...)

and the two except blocks become

except ImportError as e:
    if not self.allow_local_archive_fallback:
        raise RuntimeError(
            f"{db.name} is remote but the 'postgrest' extra is not installed"
        ) from e
    _logger.warning(...)
except Exception as e:
    if not self.allow_local_archive_fallback:
        raise RuntimeError(
            f"Could not create the PostgREST controller for {db.name}: {e}"
        ) from e
    _logger.warning(...)

The if server_url: guard is the important one: today it falls through to SQLite without even logging, which is the path we actually hit.

2. Do not default the scheme to http

schema = getattr(server, "schema_", None)
if schema is None:
    raise ValueError(
        f"{getattr(server, 'domain', '?')} declares no schema; refusing "
        "to guess http for what may be an https endpoint"
    )

Guessing http for a server that declares https is the kind of thing that surfaces as an obscure connection error much later, and silently downgrades transport security if the endpoint happens to answer on 80.

3. The trigger

Worth fixing regardless, but note the reason server is None here is OO-LD/oold-python#106: the declared field type and the class the resolver returns are two different classes for the same category, so pydantic re-constructs the resolved node from dict(value) instead of accepting it. IRI-only fields are None in __dict__, so the reference is dropped on the way into the cache. With that fixed, db.server resolves to a real DatabaseServer and this method builds https://<domain>:443 correctly. The changes above are still worth making so the next resolution failure is not silent.

Test

A test that constructs a DataTool with a storage location whose server cannot be resolved, and asserts it raises rather than returning a controller pointing at a local file.