Pylons / pyramid_blogr

Pyramid_blogr is an example implementation of Flaskr app with Pyramid Web Framework
72 stars 39 forks source link

Refactor to feature context and route factories #53

Closed pauleveritt closed 7 years ago

pauleveritt commented 8 years ago

@ergo This ticket is to give you a heads-up on a direction we're going.

As discussed in email, getting a context via a route factory is a great way to both simplify views and tap into powerful framework features. It's a significant advantage for Pyramid, one we should showcase. This repo has been on a stead march to show "best practices" in Pyramid+SQLAlchemy, so let's start here.

This is just kind of an uber-ticket, to record a conversation. I will break it into individual steps, each with a ticket and PR. While I think I know what I'm doing, it won't surprise me if @mmerickel and @ergo correct me. :)

ergo commented 8 years ago

True, I use context factories a lot, I now that switching the flow based on context is powerful, in practice for some of views that I use in production when we have url like: report/5 we fetch a Report object that in turn is related to a Resource object that users have permissions to, so I do something like in my context factory:

def __init__(request):
    self.report = fetch_report()
    self.resource = report.resource
    self.__acl__ = resource.acls_for_user(request.user)

I wasn't sure how to showcase this in a nice way and not confuse users as to what the context can be used for.

pauleveritt commented 8 years ago

On Dec 1, 2015, at 9:18 AM, Marcin Lulek notifications@github.com wrote:

I wasn't sure how to showcase this in a nice way and not confuse users as to what the context can be used for.

As background, I was bringing this up with Michael and Steve. I did a 6h screencast series on Pyramid for O’Reilly, and wound up really emphasizing (step by step) route factories. I think I can propose a series of changes that makes “context” sound like something both easier and more powerful.

But it would mean a big refactoring of pyramid_blogr. For example, the model/service duality would be collapsed into just a model (with class methods for the unbound stuff.) The view functions would shrink down into general nothingness. :)

This might be more change than you’re up for, so we might apply this treatment elsewhere.

—Paul

ergo commented 8 years ago

Actually it was like this in past and the general consensus on the IRC channel was to introduce service approach and decouple everything from models. For big and complex applications you also have services that use multiple model objects for queries - it also helps you to avoid dependancy hell in that case - after introducting services in my opinion it was much easier to organize model code and queries when queries got separated from ORM models.

pauleveritt commented 8 years ago

Well, you still could return models.services.blog_record.BlogRecordService as the context for a route factory. But if you are comfortable with your approach, we should leave it as-is.

ergo commented 8 years ago

Maybe I'm misundestanding the idea here but that would mean you would have to fetch same object from db more than once per request I think, once in factory and once inside the view?

pauleveritt commented 8 years ago

Nope, the route factory would do the fetch and then pass into the view as "context".

ergo commented 8 years ago

hmm... but you stated that it could pass the BlogRecordService - not the actual BlogRecord? Maybe you meant the ORM object :)

pauleveritt commented 8 years ago

Ah, sorry, I might be misunderstanding your misunderstanding. :) Can you explain "you would have to fetch same object from db more than once per request I think, once in factory and once inside the view"?

ergo commented 8 years ago
Well, you still could return models.services.blog_record.BlogRecordService as the context...

I think you meant that we should return BlogRecord. And that would work well indeed.

url: /objectDType/55 It's just that in my experience you fetch object D that belongs to A(because user has permissions set on A) and you have to go via objects B and C to get there, and I like to return ALL of them at once as view context to avoid reasking the db for them again - because often views operate on multiple objects.

But what you suggest works in many cases, I just like the idea of showing people that context object can be a container of "anything" :-)

pauleveritt commented 8 years ago

On Dec 1, 2015, at 3:18 PM, Marcin Lulek notifications@github.com wrote:

Well, you still could return models.services.blog_record.BlogRecordService as the context... I think you meant that we should return BlogRecord. And that would work well indeed.

url: /objectDType/55 It's just that in my experience you fetch object D that belongs to A(because user has permissions set on A) and you have to go via objects B and C to get there, and I like to return ALL of them at once as view context to avoid reasking the db for them again - because often views operate on multiple objects.

Hmm, it sounds like you are describing a resource tree and hierarchical security. I was intentionally leaving out that discussion and focusing on the case most non-Pyramid people think about…getting the resource for a route.

In your description above, is 55 related to C->B->A in a consistent way, or are these ACL hierarchies all ad-hoc?

Actually, let’s drop this conversation. I think you’d like pyramid_blogr to continue doing object lookups in views, and it’s your package, so you win. :)

—Paul

ergo commented 8 years ago

That depends, about 80% of time single ORM object is the context of factory in my code - like you suggest, however it makes sense to point out that context and acl is very flexible may return different results based on time of day or whatever other condition condition is met, this is a very interesting use case - to close some urls on weekend for example.

I very often use reource-less permissions like "access_admin_panel" that will not have ORM object as context.

It is perfectly valid to provide an example of object returned by factory, but it would make sense to introduce a new step that would showcase that to show people multiple approaches to the problem. I'm all open for a use case for this - maybe... user settings/profile updates? That could return user object as context - would that work?

ergo commented 7 years ago

Will reopen if needed.