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

figure out how to normalize view introspectables #2533

Open
mmerickel opened this issue Apr 26, 2016 · 2 comments
Open

figure out how to normalize view introspectables #2533

mmerickel opened this issue Apr 26, 2016 · 2 comments

Comments

@mmerickel
Copy link
Member

The permission= and config.set_default_permission are normalized in config.add_view where the default permission is actually pulled out and set as the value in info.options. This is inconsistent with the require_csrf= and config.set_default_csrf_options where if the default is used then require_csrf does not show up in the introspectable (not good). Unfortunately if we normalize the value in add_view then:

  1. It's not possible for anyone else to do this except core.
  2. It's no longer possible to tell if require_csrf was set on the view or not. This is necessary to determine whether to enforce the checks on the exception view, which are only used if the user explicitly sets the value.

I'm currently thinking it may be necessary to define info.raw_options versus info.options or something, to tell exactly what options were passed to the config.add_view versus which ones were normalized. However, again, this normalization is something only Pyramid can do (inside config.add_view), which is not ideal when thinking about extension authors.

We could also stop normalizing permission. However, I'd still want the introspectable to show the final value. How to do this? We could put the introspectable in the view deriver info, and allow the derivers to write to it. So far that's the best option I have.

@mmerickel
Copy link
Member Author

mmerickel commented Apr 26, 2016

Well after digging a bit deeper I discovered that the introspectable did not actually show the final value, which means I was able to refactor the logic to work exactly like csrf does. #2534 demonstrates this.

The second part of this issue is still valid which is that it'd be nice if view derivers could affect the introspectable. If they could, we would be able to move a fairly large chunk of logic out of config.add_view and into the derivers themselves. This would allow config.add_view to focus almost exclusively on the crazy MultiView it is constructing, and less so on bookkeeping around derivers. For example, almost all of this junk at the end could be in the individual derivers:

if mapper:
mapper_intr = self.introspectable(
'view mappers',
discriminator,
'view mapper for %s' % view_desc,
'view mapper'
)
mapper_intr['mapper'] = mapper
mapper_intr.relate('views', discriminator)
introspectables.append(mapper_intr)
if route_name:
view_intr.relate('routes', route_name) # see add_route
if renderer is not None and renderer.name and '.' in renderer.name:
# the renderer is a template
tmpl_intr = self.introspectable(
'templates',
discriminator,
renderer.name,
'template'
)
tmpl_intr.relate('views', discriminator)
tmpl_intr['name'] = renderer.name
tmpl_intr['type'] = renderer.type
tmpl_intr['renderer'] = renderer
introspectables.append(tmpl_intr)
if permission is not None:
# if a permission exists, register a permission introspectable
perm_intr = self.introspectable(
'permissions',
permission,
permission,
'permission'
)
perm_intr['value'] = permission
perm_intr.relate('views', discriminator)
introspectables.append(perm_intr)

@mmerickel
Copy link
Member Author

I might need to abandon my hopes of having view derivers register their own introspectables. The initial thought was to add info.view_intr which the derivers could use to relate their own introspectables to the view. I've been staring at it for a while now and there are a few issues.

  1. There may not be a view_intr to relate to in the case of calling config.derive_view directly instead of coming from config.add_view. In this case no introspectables can be registered.
  2. The list of introspectables is actually supposed to exist prior to the action callable. The api is config.action(discriminator, callable, ..., introspectables=[...]). The derivers are not executed until the callable is executed. It just so happens that the order works out such that we could mutate the introspectables list from inside the callable but damn it feels weird, don't it? Not the end of the world though, and hidden from the user.
  3. If we ignore 2 we still need a way for the deriver to create an introspectable and tell us to add it to the list. info.introspectables.append(...) is about all I have right now, but I liked it when info was more immutable. Oh well.
  4. Currently to create an introspectable you use the config object to call intr = config.introspectable(...) which is an alias for intr = pyramid.registry.Introspectable(...). The class is hidden for some reason. Where would the deriver get the factory from? intr = info.introspectable(...)?

No show stoppers, but it doesn't exactly feel like it's falling into place either.

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

No branches or pull requests

1 participant