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

Add support for different Cache-Control/Expires headers for S3 #170

Merged
merged 10 commits into from Dec 30, 2014
Merged

Add support for different Cache-Control/Expires headers for S3 #170

merged 10 commits into from Dec 30, 2014

Conversation

bulyshko
Copy link
Contributor

@bulyshko bulyshko commented Sep 3, 2014

See #167

@BanzaiMan
Copy link
Contributor

A few things.

  1. Indent with 2 spaces
  2. We need specs
  3. What does the invocation look like? How will be configured in .travis.yml, and how would it be invoked on the command line?

@bulyshko
Copy link
Contributor Author

bulyshko commented Sep 3, 2014

  1. Oops! Fixed.
  2. I'll add specs later today.
  3. Please see Add support for different Cache-Control headers for S3 deploy  #167 (comment) for .travis.yml example. Command line arguments could look like this:
--cache_control="max-age=2592000"
--cache_control="no-cache: index.html"
--expires="'2015-01-01 00:00:00 -0000': 'assets/**/*.{js,css}'"
--expires="'2020-01-01 00:00:00 -0000': '**/*.png', '**/*.jpg', '**/*.gif'"

@bulyshko
Copy link
Contributor Author

bulyshko commented Sep 4, 2014

@BanzaiMan please let me know if you have any thoughts regarding command line.

@Joshua-Anderson
Copy link
Contributor

@BanzaiMan Does .travis.yml hash compacting work now? I seem to remember a PR for traivis build at one point. Does travis-yaml work properly with hashes?

@BanzaiMan
Copy link
Contributor

@Joshua-Anderson I am not sure.

@bulyshko Does that command line option work? And I don't think the example you provide is getting exercised in the spec. I am also not sure if this is going to work in general. What happens if more than one glob match a single file? What takes precedence?

What date time format is valid here? Is it safe to pass whatever user gives?

@rkh
Copy link
Contributor

rkh commented Sep 5, 2014

You mean hashes in hashes? I think both travis-build and travis-yaml currently interpret that as branch specific options.

Modelling each provider's options in travis-yaml would solve this.

@bulyshko
Copy link
Contributor Author

bulyshko commented Sep 5, 2014

@BanzaiMan,

Does that command line option work?

Command line functionality is not implemented yet. I'm still waiting for your suggestions on how it should look like.

I don't think the example you provide is getting exercised in the spec.

I can add more specs any time if you think we need to check something else. Currently, it covers every scenario I could imagine.

I am also not sure if this is going to work in general.

Please tell me more. What makes you think so?

What happens if more than one glob match a single file? What takes precedence?

If more than one glob matches a single file the last match will be used. Same happens for this example:

cache_control:
- max-age=2592000
- max-age=300

Cache-Control will be set to max-age=300.

What date time format is valid here? Is it safe to pass whatever user gives?

Why should we care about date time format at all? I mean currently docs say: The date must be in the format YYYY-MM-DD HH:MM:SS -ZONE. But this is not validated by the code and I don't think it should be. See s3.rb (line 43).

For us it is safe to pass whatever user gives.

@BanzaiMan
Copy link
Contributor

My doubt about "working in general" refers to collisions that I mentioned afterwards. There should be documentation and spec for this case.

I didn't look up the date format specification. I understand that now.

BanzaiMan added a commit that referenced this pull request Dec 30, 2014
Add support for different Cache-Control/Expires headers for S3
@BanzaiMan BanzaiMan merged commit 8b88c37 into travis-ci:master Dec 30, 2014
@nsilberman
Copy link

Hi
just a quick question.
If you want to only have a max-age cache_control for a specific file, what is the syntax for .tavis.yml. This one doens't seem to work :

deploy: 
  cache_control:
    - max-age=60:
      - "init.js"

The example in the doc is only for the no-cache parameter

@nsilberman
Copy link

ping @BanzaiMan and @bulyshko
many thanks guys !

@bulyshko
Copy link
Contributor Author

@nsilberman Thank you for your comments. I've tested myself (https://github.com/bulyshko/website/commit/3d920ed0132816ab7e3bf34fa4fde9a5c85e0cec) and it seems you are right - it's not working. I'm not sure if my changes were pushed to production. CC @BanzaiMan

@nsilberman
Copy link

Don't know. So for the moment, the only way to do this is a little bit tricky :

before_install:
- openssl aes-256-cbc -K $encrypted_ENCRYPTIONKEY_key -iv $encrypted_ENCRYPTKEY_iv
  -in credentials.enc -out .aws/credentials -d
- mv .aws ~/
- sudo pip install awscli
after_deploy:
- aws s3api copy-object --bucket BUCKETNAME --copy-source BUCKETNAME/FILENAME
  --cache-control max-age=60 --metadata-directive REPLACE --key FILENAME

@bulyshko
Copy link
Contributor Author

Also you can use AWS Lambda for this.

@nsilberman
Copy link

great idea @bulyshko

@tlvince
Copy link

tlvince commented Mar 26, 2016

Has this been deployed? I've tried:

  edge: true
  cache_control:
    - max-age=2592000
    - no-cache:
      - index.html

… and can see cache-control: max-age=2592000 in all my bucket's files, including index.html; I expected cache-control: no-cache on index.html.

@klaemo
Copy link

klaemo commented May 13, 2016

yep I can confirm that the whitelisting doesn't work :(

@bulyshko
Copy link
Contributor Author

Will take a look.

@danhalliday
Copy link

Does this work in travis.yml? I can't seem to get patterns working:

cache_control:
  - public
  - max-age=31536000: '**/*'
  - max-age=30: '**/*.html'

@danielwhatmuff
Copy link

Any news on this? Would be great to have it working.

@BanzaiMan
Copy link
Contributor

See #486 for further discussion. I believe the feature is incomplete. Sorry for the tardy notice here.

@bulyshko
Copy link
Contributor Author

Hi @BanzaiMan, thanks for pinging me. I’ve found a problem. Will try to address in the next few days.

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.

None yet

9 participants