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

refactor dispatch #387

Closed
wants to merge 14 commits into from
Closed

refactor dispatch #387

wants to merge 14 commits into from

Conversation

chadwhitacre
Copy link
Contributor

Refactor the dispatcher so that it doesn't depend on Aspen's request, response and website objects. I want to be able to use the dispatcher inside of Django.

Builds on #385/#386.

Instead of overloading request.headers for implicit content negotiation
and autoindexing, store the needed info in a dispatch_result.extra dict,
passing dispatch_result around as needed.
@chadwhitacre
Copy link
Contributor Author

@pjz I have more to do here to fully modularize the dispatcher (removing references to Aspen request, response, and website objects), but the commits so far should leave the code in a stable state if you want to merge sooner rather than later.

@chadwhitacre
Copy link
Contributor Author

Blech. Broke the tests. :-/

Outta steam for now, will pick up with this again later ...

@pjz
Copy link
Contributor

pjz commented Oct 3, 2014

I like the way this is trending. I've pondered trying to make the dispatcher usable by Flask before (since that's a very pure-WSGI stack); that might also be a worthy goal.

@chadwhitacre chadwhitacre changed the title [WIP] dispatch refactor dispatch refactor Oct 3, 2014
@chadwhitacre
Copy link
Contributor Author

Okay! Ready for review, @pjz. :-)

I decided not to take out the dependency on Response. We're not trying to factor dispatcher entirely out of Aspen. A Django or Flask adapter will need to translate raised exceptions for each framework, and catching Response is as good as anything else. Raising fine-grained exceptions is #304; I think that's out of scope here.

This was referenced Oct 3, 2014
@chadwhitacre chadwhitacre changed the title dispatch refactor refactordispatch Oct 3, 2014
@chadwhitacre chadwhitacre changed the title refactordispatch refactor dispatch Oct 3, 2014
@chadwhitacre
Copy link
Contributor Author

Closing in favor of #388.

@chadwhitacre chadwhitacre deleted the dispatch-refactor branch October 3, 2014 18:46
@chadwhitacre chadwhitacre restored the dispatch-refactor branch October 3, 2014 18:46
@chadwhitacre chadwhitacre mentioned this pull request Oct 3, 2014
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.

2 participants