Feature: --database-name should take a URL

[xkortex]

I'm looking to tinker with using SAPP with multiple users, so I'd like to be able to run with some client/server style database (I'm thinking postgres or possibly python-dqlite. ). However the DB class does not handle URLs directly. It is typically in SQLAlchemy to use a URL to define all the connection parameters of a database. The DBType enum is somewhat superfluous, since the driver can be inferred from a proper URL. It would be pretty straightforward to refactor it to pass in a URL and just pass that directly to sqlalchemy.engine.url.make_url, and fall back if it's a file path.

If you wanted to keep the same interface, you could have a helper function something like

class DBType(Enum):
    XDB = "xdb"  # not yet implemented
    INFER = "infer" 
    SQLITE = "sqlite"
    MEMORY = "memory"


def _make_url(name_or_url: Optional[Union[str, sqlalchemy.engine.url.URL]] = None, 
              dbtype: Union[DBType, str] = DBType.INFER,
              default_db_file: str = 'sapp.db') -> sqlalchemy.engine.url.URL:
    if dbtype is DBType.MEMORY or name_or_url == ':memory:':
        return sqlalchemy.engine.url.URL('sqlite', database=":memory:")
    if dbtype is DBType.SQLITE:
        return sqlalchemy.engine.url.URL('sqlite', database=name_or_url or default_db_file)
    if dbtype is DBType.INFER:
        return sqlalchemy.engine.url.make_url(name_or_url)

    raise errors.AIException(f'unsupported database type: {dbtype}')

This would keep the existing CLI behavior exactly as is, while allowing folks to pass in different URLs. Obviously plugging into postgres would require a bit more tooling (seems there is some graphene integration which relies on some particular functions/stored procedures - it looks tractable) but I think this would be a start in the right direction. It's a useful feature in its own right - URLs are a pretty standard way to deal with database connections. Caveat: I'm not sure what XDB is. Also is there any reason why DBType is a sqlalchemy.Enum as opposed to enum.Enum?

I could PR this if you want. Just sketched this idea out and tests pass.

[0xedward]

Hey @xkortex, so sorry about the late reply! :(

Thanks for the suggestion! I think if we wanted to implement this, we probably want to keep the current interface the same.

cc @dkgi and @arthaud, what are your thoughts on this?

[arthaud]

I would accept that PR personally.