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

use page 1 metadata for allowed methods #204

Open
chadwhitacre opened this issue Jun 27, 2013 · 13 comments
Open

use page 1 metadata for allowed methods #204

chadwhitacre opened this issue Jun 27, 2013 · 13 comments

Comments

@chadwhitacre
Copy link
Contributor

Right now we have a request.allow method that raises 405 if the method isn't in *args. What if we did this declaratively in page 1 rather than programmatically in page 2?

allowed_methods = ['GET', 'HEAD']
[---]
[---]

That would be the default, meaning simplates would have to explicitly opt in to non-idempotent methods. That smells more secure to me, along the lines of explicitly exporting variables from page 2 into the page 3 template context (#165).

It also moves us in the direction of using metadata in page 1 to influence request handling. I like that direction, somehow.

@pjz
Copy link
Contributor

pjz commented Jun 27, 2013

I like the idea - it puts control where it should be, in the page.

What I don't like is More Magic Names. Can we namespace it somehow? what about putting all this stuff on something called 'self' ? (thus referring to the simplate/endpoint)

self.allowed_methods = [ 'GET', 'HEAD' ]

Actually seems clearer to me than without the 'self'.

@chadwhitacre
Copy link
Contributor Author

Would self point to the resource object representing the simplate?

@chadwhitacre
Copy link
Contributor Author

And would we call it self or could we call it resource?

@chadwhitacre
Copy link
Contributor Author

We already have resource in the simplate context.

@chadwhitacre
Copy link
Contributor Author

So it sounds like we would use resource.allowed_methods for this.

@Changaco
Copy link
Member

A simple variable was my original idea too (liberapay/liberapay.com#116), but that wouldn't alert the developer if they type the name wrong. The simplest solution to that would be to do:

+resource.allow('GET', 'POST')
 [---]
-request.allow('GET', 'POST')

But there's another issue: should the above disallow HEAD? Currently it does because request.allow() doesn't treat HEAD specially. We could automatically allow HEAD when GET is allowed, but what if you really want to disallow HEAD for some crazy reason?

I guess the simplest solution to that would be to have two methods, e.g. also_allow and only_allow.

@chadwhitacre
Copy link
Contributor Author

that wouldn't alert the developer if they type the name wrong.

It could, right?

for method in allowed_methods:
    if method not in ALL_METHODS:
        raise Warning('Unusual verb')

@Changaco
Copy link
Member

:D I meant the name of the variable, not the HTTP methods.

@chadwhitacre
Copy link
Contributor Author

🐭

@Changaco
Copy link
Member

Another question just came to me: do we want this to be in Pando, or in Aspen? I guess it would be better in Aspen because then the Django and Flask plugins could benefit from it. Note that Aspen currently doesn't have access to the method, we would need to put it in the state dict.

@chadwhitacre
Copy link
Contributor Author

Is there a way that Aspen can delegate to the framework/plugin, so that it doesn't need to know the method? (Is the Resource object in Pando or Aspen? ... looks like that's duplicated?)

@Changaco
Copy link
Member

Changaco commented Aug 1, 2016

Is there a way that Aspen can delegate to the framework/plugin, so that it doesn't need to know the method?

I suppose Aspen could provide an algorithm function that isn't in its own algorithm by default but instead is meant to be inserted into it by Pando and others. That would avoid the code duplication without actually requiring that the method be passed to Aspen.

(Is the Resource object in Pando or Aspen? ... looks like that's duplicated?)

The latter doesn't exist anymore on the use-aspen branch.

@chadwhitacre
Copy link
Contributor Author

The latter doesn't exist anymore on the use-aspen branch.

Okay, so resource remains as part of Aspen. I suspect we'll hit this question a few times as the new architecture shakes out: how to manage the boundary between Aspen and the frameworks. I say go for whatever right now and we can reassess when we have more experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants