Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Add SQLAlchemy storage #527

Merged
merged 1 commit into from
Jun 29, 2016
Merged

Add SQLAlchemy storage #527

merged 1 commit into from
Jun 29, 2016

Conversation

miedzinski
Copy link
Contributor

#319, #345

Heavily inspired by oauth2client.contrib.django_orm.

@miedzinski miedzinski mentioned this pull request Jun 23, 2016
Closed
@miedzinski miedzinski force-pushed the sqlalchemy-storage branch 2 times, most recently from f303d5b to 46481b0 Compare June 23, 2016 20:56
theacodes
theacodes reviewed Jun 23, 2016
View reviewed changes
oauth2client/contrib/sql_alchemy.py
from sqlalchemy.types import PickleType


__author__ = '[email protected] (Dominik Miedzinski)'

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@theacodes theacodes self-assigned this Jun 23, 2016
@miedzinski miedzinski force-pushed the sqlalchemy-storage branch from 46481b0 to 77ecb62 Compare June 23, 2016 21:13
theacodes
theacodes reviewed Jun 23, 2016
View reviewed changes
oauth2client/contrib/sql_alchemy.py
"""OAuth 2.0 utilities for SQLAlchemy.
Utilities for using OAuth 2.0 in conjunction with SQLAlchemy.
Heavily inspired by equivalent Django module.
"""

This comment was marked as spam.

Sign in to view

@miedzinski miedzinski force-pushed the sqlalchemy-storage branch from 77ecb62 to f5b406e Compare June 23, 2016 22:23
@miedzinski
Copy link
Contributor Author

I have pushed new changes, as suggested. Note that this code was heavily inspired by existing Django module (including coding style and documentation quality).

@miedzinski miedzinski force-pushed the sqlalchemy-storage branch 4 times, most recently from e2884d7 to 78ba5a0 Compare June 23, 2016 23:38
@theacodes
Copy link
Contributor

Note that this code was heavily inspired by existing Django module (including coding style and documentation quality).

I'm sorry you give you the impression of inconsistency here - but we are holding new changes to a higher standard than existing code. @waprin is working on re-writing the django modules to better fit our new standards and documentation quality.

theacodes
theacodes reviewed Jun 24, 2016
View reviewed changes
oauth2client/contrib/sql_alchemy.py
"""OAuth 2.0 utilities for SQLAlchemy.

Utilities for using OAuth 2.0 in conjunction with SQLAlchemy.
Heavily inspired by equivalent Django module.

This comment was marked as spam.

Sign in to view

theacodes
theacodes reviewed Jun 24, 2016
View reviewed changes
oauth2client/contrib/sql_alchemy.py
"""Write a Credentials to the SQLAlchemy datastore.

Args:
credentials: Credentials, the credentials to store.

This comment was marked as spam.

Sign in to view

@theacodes
Copy link
Contributor

@miedzinski this is getting close, most of my nits are just around documentation and such. Thank you for this.

Once I'm satisfied, one of the other reviewers will do a final review. :)

theacodes
theacodes reviewed Jun 24, 2016
View reviewed changes
tests/contrib/test_sqlalchemy.py
self.assertTrue(q.first() is None)


if __name__ == '__main__': # pragma: NO COVER

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@miedzinski miedzinski force-pushed the sqlalchemy-storage branch 2 times, most recently from bd88503 to dbd0fdb Compare June 24, 2016 21:03
@miedzinski
Copy link
Contributor Author

Done.

@theacodes
Copy link
Contributor

I'm reasonably satisfied. @nathanielmanistaatgoogle do you want to take a look?

@theacodes
Copy link
Contributor

@nathanielmanistaatgoogle seems to be very busy, @miedzinski please feel free to bug me tomorrow if he hasn't responded. Since this lives in contrib, I'd be okay with merging it.

@miedzinski miedzinski force-pushed the sqlalchemy-storage branch from dbd0fdb to c31ff3d Compare June 28, 2016 20:27
nathanielmanistaatgoogle
nathanielmanistaatgoogle reviewed Jun 28, 2016
View reviewed changes
oauth2client/contrib/sqlalchemy.py
Returns:
A :class:`oauth2client.Credentials` instance or `None`.
"""
credential = None

This comment was marked as spam.

Sign in to view

@nathanielmanistaatgoogle
Copy link
Contributor

Done with my first round of review.

@miedzinski miedzinski force-pushed the sqlalchemy-storage branch from c31ff3d to 5048d83 Compare June 28, 2016 21:57
@miedzinski
Copy link
Contributor Author

There you go.

@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 267bbc5 into googleapis:master Jun 29, 2016
@theacodes theacodes mentioned this pull request Jul 28, 2016
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@theacodes theacodes

Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@miedzinski @theacodes @nathanielmanistaatgoogle @googlebot