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

ODBC Driver 17 for SQL Server doesn't work with multidb #328

Closed
RRGTHWAR opened this issue Apr 14, 2020 · 18 comments
Closed

ODBC Driver 17 for SQL Server doesn't work with multidb #328

RRGTHWAR opened this issue Apr 14, 2020 · 18 comments
Labels

Comments

@RRGTHWAR
Copy link

I'm trying to change my project from a single database to multiple databases. I'm still building it, so it doesn't matter if I lose all of my data. But every time I do a flask db init --multidb and then try to run a flask db migrate, I get the below error:

sqlalchemy.exc.OperationalError: (pyodbc.OperationalError) ('08001', '[08001] [Microsoft][ODBC Driver 17 for SQL Server]Neither DSN nor SERVER keyword supplied (0) (SQLDriverConnect)')

If I run the init again without the multidb tag, everything works.

Looking at the below item, it seems like it might be an SSL issue, but I'm not sure how to use any of the solutions, since I'm on a Windows computer.
mkleehammer/pyodbc#610

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Apr 14, 2020

This is a SQLAlchemy issue, unfortunately I am not related with that project, and I don't have access to a SQL Server database to try this.

@RRGTHWAR
Copy link
Author

OK, understood. Thanks!

@tomas223
Copy link

tomas223 commented May 17, 2020

I was able to solve this by workaround. I didn't dig much into it and but this looks to me like Flask-Migrate or Alembic issue because I don't have any issues with SQLAlchemy or Flask itself with this connection string.

I edited migrations/env.py file:

import os
import urllib

DB_CREDENTIALS = os.environ.get("ENV_DB_CREDENTIALS")
SQLALCHEMY_DATABASE_URI = (
    "mssql+pyodbc:///?odbc_connect=%s" % urllib.parse.quote_plus(DB_CREDENTIALS)
)

config.set_main_option(
    "sqlalchemy.url",
    SQLALCHEMY_DATABASE_URI.replace("%", "%%"),
)

Also SqlServer Connection string can't contain Trusted_Connection=Yes for some reason.

@miguelgrinberg what you think?

@zzzeek
Copy link

zzzeek commented May 18, 2020

This is a SQLAlchemy issue, unfortunately I am not related with that project, and I don't have access to a SQL Server database to try this.

I cannot replicate any SQLAlchemy or Alembic issue with this. the URL passed to create_engine() as well as used within env.py directly has no issue.

it appears that SQLALCHEMY_DATABASE_URI may be being passed through various channels including "click" in order for it to be received by Alembic? is it passed as an environment variable to the alembic commandline ? are there points where escaping of some kind may be getting involved ?

@miguelgrinberg
Copy link
Owner

Hi Mike.

The env.py that I'm currently using fishes out the database connection URL from the engine object stored in Flask-SQLAlchemy's db object:

config.set_main_option(
    'sqlalchemy.url',
    str(current_app.extensions['migrate'].db.engine.url).replace('%', '%%'))

Before this version was introduced, I obtained the URL directly from Flask's config object:

config.set_main_option(
    'sqlalchemy.url', current_app.config.get(
        'SQLALCHEMY_DATABASE_URI').replace('%', '%%'))

So as you see there are no additional layers that could mangle the URL. But I should note that the issue with the % addressed with the replace call feels similar. This was a problem with Alembic's ConfigParser object, which could not take URLs with percent-encoded characters unless the percent was escaped by doubling it. (reference: #59). I wonder if there are other limitations that need to be considered.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented May 18, 2020

@zzzeek actually, I believe I may have hinted at the problem. The replace('%', '%%') was not for percent-encoded strings. It was for people who had a % in their passwords. So now we have actual % for percent-encoded characters, and I'm doubling them too.

So really the problem is that I need to pass an exact database URL to Alembic that is accepted as is, and ConfigParser complicates this. I'll see if I can figure out a way to bypass the percent encoding validation.

EDIT: after reading the docs on ConfigParser, I'm not sure, it would appear as if percent-encoded characters would also need the % doubled to be accepted, so I don't know, I don't see a clear problem with how I'm handling the URL.

@zzzeek
Copy link

zzzeek commented May 18, 2020

ConfigParser in Python needs percent signs that are passed as-is to be escaped because it uses percent-sign interpolation:

https://docs.python.org/3/library/configparser.html#interpolation-of-values

if someone has a percent sign in their password, they need to URL encode that before creating a SQLAlchemy URL, this is described at: https://docs.sqlalchemy.org/en/13/core/engines.html#database-urls

so from my end, if someone has a programmatic URL in their program that flask-migrate is going to pass to the Alembic config, it should double the percent signs as this user is doing for their workaround, and that should be it.

I'm not fully understanding what flask-migrate is trying to do with passwords.

@zzzeek
Copy link

zzzeek commented May 18, 2020

also #59 looks like exactly this issue, I'm not looking at any current code right now so I dont understand why there's still a problem.

@miguelgrinberg
Copy link
Owner

@zzzeek that fix went in about a year ago, so yeah I agree, it would appear this should have been handled unless the user was running a very old Flask-Migrate.

@zzzeek
Copy link

zzzeek commented May 18, 2020

OK but the "fix" impacts a generated env.py that isn't upgraded when users upgrade, so they have to edit the file manually right? that would be the full explanation here.

@miguelgrinberg
Copy link
Owner

It looks like you are discussing the issue not with the OP, but with @tomas223, which has not provided any background information to me. The OP stated that they were running the init command, so the explanation that they have an old env.py does not fit their report.

@zzzeek
Copy link

zzzeek commented May 18, 2020

agree, there's several ppl commenting on my issue and this one so for the moment I'm leaning towards a stale env.py as the problem with the user I have on sqlalchemy/sqlalchemy#5332. I think we are done for now.

@clabornd
Copy link

@miguelgrinberg @zzzeek

I believe the problem was that I downgraded Flask-migrate to match a system where I did not have this problem and my env.py was not overwritten after downgrade.

I am able to perform migrations successfully using:

config.set_main_option(
    'sqlalchemy.url', current_app.config.get(
        'SQLALCHEMY_DATABASE_URI').replace('%', '%%'))

but get the error described in sqlalchemy/sqlalchemy#5332 when using:

config.set_main_option(
    'sqlalchemy.url',
    str(current_app.extensions['migrate'].db.engine.url).replace('%', '%%'))

Which also causes the multiple calls to DefaultEngineStrategy.create described in that issue.

OS: macOS Mojave
Python: 3.5
SQLAlchemy: 1.3.10
Flask-Migrate: 2.5.2
Database: MS SQL Server

@miguelgrinberg
Copy link
Owner

@clabornd can I ask you to add the following prints in your env.py right above the config line?

print(current_app.config.get('SQLALCHEMY_DATABASE_URI'))
print(current_app.extensions['migrate'].db.engine.url)

Then run any flask-migrate command and check these two prints in the output. I'd like to know how these two connection URLs differ. Thanks.

@clabornd
Copy link

(1) >>> print(current_app.config.get('SQLALCHEMY_DATABASE_URI'))
mssql+pyodbc:///?odbc_connect=Driver%3D%7BODBC+Driver+17+for+SQL+Server%7D%3BServer%3Dtcp%3Amydb.database.windows.net%2C1433%3BDatabase%3Ddbname%3BUid%3Dazureuser%3BPwd%3Dmypword%21%3BEncrypt%3Dyes%3BTrustServerCertificate%3Dno%3BConnection+Timeout%3D30%3B

(2) >>> print(current_app.extensions['migrate'].db.engine.url)
mssql+pyodbc:///?odbc_connect=Driver={ODBC Driver 17 for SQL Server};Server=tcp:mydb.database.windows.net,1433;Database=dbname;Uid=azureuser;Pwd=mypword;Encrypt=yes;TrustServerCertificate=no;Connection Timeout=30;

>>> type(current_app.config.get('SQLALCHEMY_DATABASE_URI'))
<class 'str'>

>>> type(current_app.extensions['migrate'].db.engine.url)
<class 'sqlalchemy.engine.url.URL'>

Not sure if its useful, but:
current_app.extensions['migrate'].db.engine.url causes this double call of sqlalchemy.engine.strategies.DefaultEngineStrategy.create where the first call is taking an argument that matches the output of (1) and the second call is taking an argument that matches the output of (2)

@Miguelreis
Copy link

Joining in because I have the same problem. My print statements yield the same as @clabornd, and the temporary solution by @tomas223, also worked as expected. I would like to point out that this error does not occur when running the app normally, as it is able to connect to my db (although it has not been updated with flask upgrade), as seen in the stack trace below as table "user" doesn't exist. Hope this is useful in any way.

Flask-Migrate==2.5.3
Flask-SQLAlchemy==2.4.0
OS: macOS Catalina 10.15.3
Database: MS SQL Server

(...)
sqlalchemy.exc.ProgrammingError: (pyodbc.ProgrammingError) ('42S02', "[42S02] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Invalid object name 'user'. (208) (SQLExecDirectW)")
[SQL: SELECT TOP 1 [user].id AS user_id, [user]._uuid AS user__uuid, [user].username AS user_username, [user]._password_hash AS user__password_hash, [user].role AS user_role
FROM [user]
WHERE [user].username = ?]
[parameters: ('asd',)]

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented May 20, 2020

@zzzeek so the problem is related to how SQLAlchemy converts its engine.url object back into a string when the URL has percent-encoded characters.

For example, the URL sqlite:///db.sqlite?foo%3dbar=baz is parsed correctly on the way in:

>>> engine.url.query
{'foo=bar': 'baz'}

But a reconstructed URL loses the percent encoding, which is not only an inconvenience but on some URLs like this one also wrong:

>>> str(engine.url)
'sqlite:////Users/mgrinberg/Documents/app/db.sqlite?foo=bar=baz'

Flask-Migrate obtains the database URL from the engine instead of the Flask config because Flask-SQLAlchemy sometimes modifies the URL before passing it down to SQLAlchemy (they call it apply_driver_hacks() or something like that if I remember correctly). This was an issue that I had to address a while ago, to make sure that both Flask-SQLAlchemy and Flask-Migrate connect to the DB using the same parameters.

Do you think this is something that can be fixed in URL.__to_string__()?

@zzzeek
Copy link

zzzeek commented May 20, 2020

sure please post an issue at github.com/sqlalchemy/sqlalchemy and include an MCVE and we'll see if someone can work on that (or provide a PR with tests, after posting the issue) - thanks!

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

No branches or pull requests

6 participants