Fix read write

[akoumjian]

The default behavior for the FrameIndex was to always create tables, even when in read mode. This is problematic when you want to use read only volumes.

I am also simplifying the api just slightly by removing the open classmethod. Currently, there isn't anywhere we initialize FrameIndex by directly passing in an engine or a connection object. Instead we only use string initialization, so I've normalized it to just use __init__ based off db uri and the read/write mode.

[spenczar]

There's nothing wrong with these changes, but I'm a little concerned because sqlalchemy.schema.MetaData.create_all should not actually be issuing statements to create tables if they already exist. If you're opening the database in read mode, and those tables are missing, something is pretty wrong.

Is it possible that something else is going on over in adam_etl's failures?

[akoumjian]

There's nothing wrong with these changes, but I'm a little concerned because sqlalchemy.schema.MetaData.create_all should not actually be issuing statements to create tables if they already exist.

Yes, this was my first thought. Something weird is going on and checkfirst is not working as I would expect. I found the sqlalchemy source not at all easy to decipher. I believe it gets here but for some reason it returns True: https://github.com/sqlalchemy/sqlalchemy/blob/400aa8a676ba1a0a1536ae52a20caa93726525dd/lib/sqlalchemy/sql/ddl.py#L866C56-L866C65

Based off of this: https://github.com/sqlalchemy/sqlalchemy/blob/400aa8a676ba1a0a1536ae52a20caa93726525dd/lib/sqlalchemy/dialects/sqlite/base.py#L2138

If you're opening the database in read mode, and those tables are missing, something is pretty wrong.

The tables are not actually missing, which makes it curious that sqlalchemy thinks they are in this scenario.

Is it possible that something else is going on over in adam_etl's failures?

It is certainly possible there is either something odd in the adam-etl environment that would cause MetaData.create_all to not recognize the existing tables. I think it is unlikely something that can be remedied easily (probably something subtle about the mounted drive system and the way that sqlalchemy's sqlite driver is inspecting the pragma info on the tables).

Up to you if you think it's worth investigating further. I think it's better to just avoid create_all since for reasons I don't entirely understand it isn't doing something simple like CREATE TABLE <table> IF NOT EXISTS.

[spenczar]

Yes, I think what you have here is pragmatic and seems fine, so it is probably worth merging and deploying this as is, if you are sure the precovery DB is valid.