-
Notifications
You must be signed in to change notification settings - Fork 76
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
import EncryptedType correctly from sqlalchemy_utils #159
Conversation
1b9c3cf
to
82893db
Compare
setup.py
Outdated
@@ -33,6 +33,7 @@ | |||
|
|||
tests_require = [ | |||
'SQLAlchemy-Continuum>=1.2.1', | |||
'SQLAlchemy-Utils~=0.0,>=0.33.0.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is in test_require
but I think it should be in install_requires
instead (where the dependency is already). However, if you apply the above change then we don't need any changes to setup.py.
invenio_oauthclient/models.py
Outdated
@@ -30,7 +30,7 @@ | |||
from sqlalchemy.ext.mutable import MutableDict | |||
from sqlalchemy.orm import backref | |||
from sqlalchemy_utils import JSONType | |||
from sqlalchemy_utils.types.encrypted import EncryptedType | |||
from sqlalchemy_utils.types.encrypted.encrypted_type import EncryptedType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try:
# SQLAlchemy-Utils <=0.32
from sqlalchemy_utils.types.encrypted import EncryptedType
except ImportError:
# SQLAlchemy-Utils >=0.33
from sqlalchemy_utils.types.encrypted.encrypted_type import EncryptedType
In order to not force existing installations to upgrade SQLAlchemy-Utils immediately, I would go with above. EncryptedType
is being used in other modules (e.g. OAuth2Server I think), hence if not all modules are fixed and released at the same time then it will cause conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, on second look into SQLAlchemy-Utils, following should work on both versions:
from sqlalchemy_utils import EncryptedType
4a0a3de
to
25d833a
Compare
@@ -69,7 +69,7 @@ def upgrade(): | |||
sa.Column('token_type', sa.String(length=40), nullable=False), | |||
sa.Column( | |||
'access_token', | |||
sqlalchemy_utils.types.encrypted.EncryptedType(), | |||
sqlalchemy_utils.types.encrypted.encrypted_type.EncryptedType(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use the top level sqlalchemy_utils.EncryptedType
one.
This should be removed once inveniosoftware/invenio-oauthclient#159 is merged. Signed-off-by: David Caro <[email protected]>
25d833a
to
48d6f3f
Compare
077004e
to
9e36817
Compare
sqlalchemy_utils recently updated their
encrypted_type
module.EncryptedType
now needs to be imported fromsqlalchemy_utils.types.encrypted.encrypted_type
.Signed-off-by: Dinika Saxena [email protected]