omarryhan / sanic-cookies

The ultimate session management library for Sanic
MIT License
7 stars 3 forks source link

Asyncpg support (#1) #2

Closed mflaxman closed 5 years ago

mflaxman commented 5 years ago

I'm not sure you'd want to merge this, but I'm working on a project that uses Gino and I put this together and it's working for me.

Code needed to run it:

from sanic import Sanic
from gino.ext.sanic import Gino

from something_secure import DB_SETTINGS

app = Sanic()
app.config.update(DB_SETTINGS)
db = Gino() 
db.init_app(app)

interface = AsyncPG(client=db)
auth_session = AuthSession(app, master_interface=interface)

# define routes here

if __name__ == '__main__':
    app.run(host='127.0.0.1', port='8080')

I think the pure asyncpg module uses a different function call (not scalar()) to execute a SQL query but I haven't tried it, so this may be more for reference to others vs actually merging it in.

omarryhan commented 5 years ago

Cool stuff!

I actually don't mind merging any new interfaces as long as:

  1. The code is secure
  2. API will likely not need to be changed in the near future

And I think your code isn't failing any of these critieria.

I just had a look at the way AsyncPG recommends transacting with encoded JSON and it seems to be a bit different from the way Gino does it (https://magicstack.github.io/asyncpg/current/usage.html#example-automatic-json-conversion). So maybe we can have two AsyncPG interfaces one for Gino and one for pure AsyncPG?

If so, then i'd suggest maybe we can rename the interface to GinoAsyncPG or something similar? So that we have a proper name for a pure AsyncPG interface. What do you think?

If you're ok with this, then maybe we can also move the "create table statement" along with the "code needed to run" you posted with the PR and move it to the repo's README? I can take care of the rest if you don't have the time.

Thanks again! Really appreciated :)

mflaxman commented 5 years ago

👍 to everything you said!

If you don't mind making the changes you suggested and merging that would be much appreciated. I'd prefer my project be based off the official library (with upstream support changes) instead of my unmaintained fork.

omarryhan commented 5 years ago

I just released v0.4.0. Can you check if everything works fine for you?

mflaxman commented 5 years ago

It works, thanks!