betodealmeida commented on issue #25093: URL: https://github.com/apache/superset/issues/25093#issuecomment-1701291083
This is nice! In my opinion we don't need a SIP for this, since [it doesn't affect any public interfaces or major features](https://github.com/apache/superset/issues/5602). And even though it introduces a new dependency, DB engine spec dependencies are optional, so it should be fine. You can go ahead an PR it and add me as a reviewer, I'll be happy to take a look. A few things of notice: 1. I wrote a tool to test DB engine spec implementations (and SQLAlchemy dialects as well, to make sure they implement everything Superset needs). You can read more about it in https://github.com/apache/superset/pull/24918. 2. You have the option of having the DB engine spec be part of Superset, or you can distribute it in a separate package (see an example [here](https://github.com/DataJunction/dj/blob/3e2b687f376fd36687152d8f5d319b096367f291/datajunction-server/pyproject.toml#L63C1-L64) and [here](https://github.com/DataJunction/dj/blob/3e2b687f376fd36687152d8f5d319b096367f291/datajunction-server/datajunction_server/superset.py#L47)). Distributing it in a separate package means you can release new versions independently of Superset, which allows people to adopt features more quickly. But the downside is that the DB engine spec interface is not public, so it might change between major versions and break existing 3rd party DB engine specs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
