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 cache_max_age option #246

Closed
wants to merge 1 commit into from
Closed

Conversation

jdx
Copy link

@jdx jdx commented May 5, 2015

I'm trying to use sinopia behind a CDN

@rlidwka
Copy link
Owner

rlidwka commented May 9, 2015

This conflicts with no-cache proposal in #244.

What do you want CDN for? Is there a configuration option for that CDN to set a minimum max-age?

// cc: @bobzoller

@jdx
Copy link
Author

jdx commented May 10, 2015

I'm using sinopia to deliver the CLI plugins for the Heroku toolbelt. We have users around the world and connecting to a US sinopia server can be really slow.

Adding the CDN has really helped (Cloudfront) but AWS only offers a minimum max-age. I patched with this change on the server and it's worked well for me.

I think there is room to modify this PR to add the no-cache header if the caching option isn't set. Thoughts?

@jdx
Copy link
Author

jdx commented May 10, 2015

I can fix the tests and add one too if you like the idea. There might be a way to add this more cleanly too if you have any ideas.

@jdx
Copy link
Author

jdx commented May 10, 2015

I'm not sure it conflicts actually. As I understand that middleware will let something else override later in the request chain.

Though personally I would rather be explicit about setting the header rather than relying on middleware in a separate dependency, but that's clearly a style decision on your part.

@jdx jdx force-pushed the cache-control-max-age branch from 96d085b to f8ad7f2 Compare May 29, 2015 17:49
@jdx
Copy link
Author

jdx commented Jun 15, 2015

@rlidwka since my use-case is a little different than what most people need sinopia for, I ended up writing my own implementation https://github.com/dickeyxxx/elephant

@jdx jdx closed this Jun 15, 2015
@jdx
Copy link
Author

jdx commented Jun 15, 2015

thank you so much for making this project though! I used it in production for a while with great success.

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