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

WIP: blog demo, v0.4 #33

Merged
merged 6 commits into from Jul 29, 2018
Merged

WIP: blog demo, v0.4 #33

merged 6 commits into from Jul 29, 2018

Conversation

gyermolenko
Copy link
Contributor

@gyermolenko gyermolenko commented May 20, 2018

Wanted to share my "work in progress" for blog demo.

To discuss:

- readme is empty, sorry for that. If someone wants to start it locally, I will work on those notes.
- I've tried out gino for db access and so far it's been very convenient switched back to sqlalchemy core

  • this v0.1 has login, but I'm thinking about aiohttp-loginto handle registration/login/logout and stuff more easily

update:
I've tried out aiohttp-login. There are two ways to use it:

  1. use it's db schema as is, with existing AsyncpgStorage storage.
  2. create our own aiohttp_login storage implementing 7 required methods.
    As in p.1. it will also require table for login-related user fields.

At the moment I don't like either of those options.

Please share your feedback.

@@ -0,0 +1,28 @@
import pathlib
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, u need set all your settings in yml file.

import pathlib


BASE_DIR = pathlib.Path(__file__).parent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did u use BASE_DIR?

async def permits(self, identity, permission, context=None):
if identity is None:
return False
return True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async def permits(self, identity, permission, context=None):
        return identity is not None:

raise response


return {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add flake8, please.

demos/blog/db.py Outdated
loop.run_until_complete(db.gino.create_all())

loop.run_until_complete(create_sample_data())
# loop.run_until_complete(delete_sample_data())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove that, if this is not necessary



async def validate_login_form(form):
error = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when did u use error?

else:
return None

return 'error'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when does your function return 'error'?

aioredis

#dev
psycopg2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe psycopg2 should be in #database block

@login_required
@aiohttp_jinja2.template('index.html')
async def index(request):
username = await authorized_userid(request)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authorized_user*id* return username 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aiohttp_jinja2.template('index.html')
async def index(request):
username = await authorized_userid(request)
current_user = await User.query.where(User.username == username).gino.first()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

u will do it in every view, maybe make sense use middleware

@asvetlov
Copy link
Member

gino is a hidden singleton :(

@gyermolenko
Copy link
Contributor Author

updated a bit (work in progress).

But yes, gino is still a global var. Is this a no-go? If so, I can probably switch to sa core, but would like to hear more about downsides.

And maybe anybody has other general objections?

@jettify
Copy link
Member

jettify commented Jun 17, 2018

I would advise not to use Gino altogether if it support only global var way to create models...

teardown_db(executor_config=admin_db_config, target_config=user_db_config)
setup_db(executor_config=admin_db_config, target_config=user_db_config)

import asyncio
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ugly probable should be at the top?

@gyermolenko gyermolenko changed the title WIP: blog demo, v0.1 WIP: blog demo, v0.3 Jun 29, 2018
if username:
raise redirect(request.app.router, 'index')

if request.method == 'POST':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets make separate methods one for post second for get

@jettify
Copy link
Member

jettify commented Jun 30, 2018

I like changes, but flake issues should be resolved

@jettify
Copy link
Member

jettify commented Jun 30, 2018

Having aiohttp-login example is good idea

@gyermolenko gyermolenko changed the title WIP: blog demo, v0.3 WIP: blog demo, v0.4 Jul 22, 2018
@gyermolenko
Copy link
Contributor Author

@jettify

Having aiohttp-login example is good idea

I've tried it and left an update to PR's description.
Have you seen the lib being used somewhere? I thought about db relations, and first thing that comes to mind is one-to-one as in django's Extending the existing User model

@jettify
Copy link
Member

jettify commented Jul 22, 2018

lets merge this PR and do incremental udpdates

@gyermolenko gyermolenko merged commit fd1073f into master Jul 29, 2018
@gyermolenko gyermolenko deleted the blog_demo branch May 22, 2019 12:52
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.

None yet

4 participants