Rest Tunneling #1
Conversation
… RoR-like REST method tunneling functionality.
…--Dispatch into rest-tunneling
|
Tim, Thanks for following up on-list. I'm reviewing this proposal again in light of that feedback. |
|
Mark: I added the docs as requested and have (I think) push a new revision to you for integration with the master. Please tell me if there is anything else. Thanks. |
|
What's the status on this pull request? Please advise. |
|
What is the status of this pull request? I did not merge this in with my previous pull request for tracking purposes. |
|
Tim, I apologize for the slow pace of this thus far. It would have been helpful if more feedback from the community had materialized to provide input, but it did not. Here's some further feedback on it: Overall, I would like to accept some version of this feature. It seems like it could be commonly. Because I see it being used commonly, I'd like to for it be enabled with one option rather than two. So, I think either: "auto_rest" should imply "auto_rest_tunneling" or "auto_rest_tunneling" should imply "auto_rest". Do you see a problem with modifying the current behavior of "auto_rest" in this way? That seems a little cleaner, but having "auto_rest_tunneling" imply "auto_rest" also seems OK with me, and has no backcompat concerns. A second place a see for improvement is validating the method. Right now it is passed directly from the HTTP request into a method name without validation. What about first checking that it matches a known HTTP verb, and thrown an exception otherwise? Finally, we'll need some tests, but those can wait until the spec for the feature is final. |
|
We'll also need a similar patch to CGI::Application::Dispatch::PSGI, which for better or worse, duplicates some of the code in CGI::Application::Dispatch. |
There was a problem hiding this comment.
Should this be line-wrapped? Also, at end of line, there is a typo - "Ifa C<_mode> param is" should be "If a C<_mode> param is"
There was a problem hiding this comment.
Yes, this line should be wrapped. Thanks for noticing.
…--Dispatch into rest-tunneling Conflicts: lib/CGI/Application/Dispatch.pm
|
Was considering adding tunneling to the auto_rest switch, but wasn't sure how you would feel about changing its behavior. Would anyone want REST without tunneling? I don't know. I'm all for less switches and think both should be used always. So I'll get that done. I didn't realize PSGI support didn't share the effected part. I'll look at implementing that. Validation of what is passed is a good call also. Anything else? |
Yes. I had looked at having the two dispatchers (PSGI and legacy) share more code, but that looked like it was going to be rather involved, so it's more like a "fork" for now. This could be benefit for those using PSGI, as there's less interdependence with code that's not in the PSGI code path. |
|
Tim, the last status here is that you were volunteering to add tunneling to the auto_rest flag, and also update CGI::Application::Dispatch::PSGI. Are you still up for that? |
ghost
assigned
3 participants
This adds the REST tunneling feature that was discussed on cgi-app a month of so ago. I've merge it with the more recent releases that worked in PSGI support. Please let me know if there is anything else I can do for this to get release. Thanks.