Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for MSSQL server using aioodbc #151

Closed
wants to merge 5 commits into from

Conversation

ranjanashutosh
Copy link

Added support for MSSQL server using aioodbc.
Will write the test cases soon.

@ranjanashutosh
Copy link
Author

ranjanashutosh commented Oct 23, 2019

Build and check is failing because ODBC driver is not installed on the machine.

mkleehammer/pyodbc#276

https://docs.microsoft.com/en-us/sql/connect/odbc/linux-mac/installing-the-microsoft-odbc-driver-for-sql-server?view=sql-server-ver15

@CommonCrisis
Copy link

Is this PR dead?

@gvbgduh
Copy link
Member

gvbgduh commented Feb 5, 2020

Great effort! That looks good, but there're a few points I wonder about.
aioodbc seems to be a generic driver and isn't explicitly tightened to MS SQL and I it can be used for say Oracle as well.
With that, it might have the sense to have the back-end implementation generic, but have the specific alias to the back-end, like

class Database:
    SUPPORTED_BACKENDS = {
        "postgresql": "databases.backends.postgres:PostgresBackend",
        "mysql": "databases.backends.mysql:MySQLBackend",
        "sqlite": "databases.backends.sqlite:SQLiteBackend",
        "mssql": "databases.backends. aioodbc:AIOODBCBackend",
        # And potentially
        "oracle": "databases.backends. aioodbc:AIOODBCBackend",
        "aioodbc":  "databases.backends. aioodbc:AIOODBCBackend",
    }

so it can cover a range of DBs.

@tomchristie
Copy link
Member

There's a nice chunk of fantastic work here. However I think we're going to need to push for figuring out having this as a third party option, rather than supported in core. I'd really like the core to be limited to postgresql, mysql, and sqlite. Any database options that I'm not able to easily test locally are going to be problematic, and in any case it'd be really helpful for us to be able to push driver implementations towards third party support, to help with long term maintenance.

Very happy to talk any of this over as needed on https://gitter.im/encode/community

@sachabest
Copy link

Hi @tomchristie - would you be interested in discussing here the rationale behind declining support for SQL Server? I believe if the limiting factor is mainly local testing, you may wish to take a look at one of the SQL Server docker images. Those should provide you a way to (EULA permitting, of course, which in this case seems like a clear cut 👍) spin up a local database and test at will. Of course these are not 100% accurate replications of SQL Server running on Windows Server 2xxx, but such images should provide ample test surface.

It seems rather arbitrary to decline support for one database, but if the rationale is lack of ability to test, that makes total sense. That said, hopefully others can benefit from the great work done in this pull request, at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants