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

added proxy cache resolver #318

Merged
merged 3 commits into from
Feb 7, 2014
Merged

added proxy cache resolver #318

merged 3 commits into from
Feb 7, 2014

Conversation

digitalkaoz
Copy link
Contributor

see the attached doc for more details...

tldr;

adds the ability to use any resolver within the Proxy Cache-Resolver

see the docs for more details

@makasim
Copy link
Collaborator

makasim commented Feb 6, 2014

could it decoupled from aws s3. It could be a decorator. Same appoach we used for CacheResolver.

@makasim
Copy link
Collaborator

makasim commented Feb 6, 2014

Would it be possible to open a PR for develop branch?

@digitalkaoz
Copy link
Contributor Author

ok, now completly decoupled from any Resolver, acts as transparent Decorator for every other Resolver

@digitalkaoz
Copy link
Contributor Author

mh symfony 2.0 fails, should we deprecate 2.0? could provide a patch, but is it good to support such an old versions long term? //cc @lsmith77

@havvg havvg added this to the v0.18.x milestone Feb 7, 2014
@havvg havvg added the Feature label Feb 7, 2014
@makasim
Copy link
Collaborator

makasim commented Feb 7, 2014

@digitalkaoz FYI develop branch (which would become 1.0 in near future) requires ~2.3 version.

@digitalkaoz
Copy link
Contributor Author

what means near future? :)


private function rewriteResponse($response)
{
if ($response instanceof RedirectResponse && $this->hosts) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on develop branch, resolve method always return an url as a string.

@makasim
Copy link
Collaborator

makasim commented Feb 7, 2014

what means near future? :)

when all release blockers will be solved and we are confident it is stable (I mean interfaces, the code already stable)

There were huge changes already done: master...develop, you could look at UPGRADE.md file too

@makasim
Copy link
Collaborator

makasim commented Feb 7, 2014

I would really advice you to impl this feature over develop branch.

@havvg
Copy link
Contributor

havvg commented Feb 7, 2014

mh symfony 2.0 fails, should we deprecate 2.0? could provide a patch, but is it good to support such an old versions long term? //cc @lsmith77

You could use the headers directly $response->headers->get('Location'); instead of $response->getTargetUrl.

@digitalkaoz
Copy link
Contributor Author

mh ok, what if i make this one compatible with master and provide a patch for develop once rebased? i would really love to see this feature merged as soon as possible (pushing things upstream from my current project, so the actual code-base shrinks, and i have to write fewer Tests ;) )

@@ -0,0 +1,106 @@
<?php


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply the coding conventions. There are several empty lines duplicated. The file endings are wrong.

@havvg
Copy link
Contributor

havvg commented Feb 7, 2014

It's ok to me. The only thing not happening are BC breaks. Small features (such as filters, resolvers, ..) are absolutely fine to be merged into master, too.

@digitalkaoz
Copy link
Contributor Author

line endings? i dont think so, i have a unix computer ;) but will investigate and make i compatible with 2.0

btw, can we tag the current master? :)

@havvg
Copy link
Contributor

havvg commented Feb 7, 2014

No, not line endings, but file endings (empty line at the end missing).

@havvg
Copy link
Contributor

havvg commented Feb 7, 2014

btw, can we tag the current master? :)

tagged v0.19.0

@havvg havvg mentioned this pull request Feb 7, 2014
@digitalkaoz
Copy link
Contributor Author

ok travis thinks its ok now :)

havvg added a commit that referenced this pull request Feb 7, 2014
@havvg havvg merged commit 5598514 into liip:master Feb 7, 2014
@digitalkaoz
Copy link
Contributor Author

can we tag it again? :) ah already done :) thank you :)

@digitalkaoz digitalkaoz deleted the s3_proxy_cache branch February 7, 2014 11:29
@havvg
Copy link
Contributor

havvg commented Feb 7, 2014

Already done, v0.20.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Level: New Feature 🆕 This item involves the introduction of new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants