Skip to content

Keep modification time when zipping single file to avoid new checksum #3436

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

Closed
wants to merge 1 commit into from

Conversation

henrist
Copy link

@henrist henrist commented Jul 9, 2018

The cloudformation package command will upload a zip, e.g. for source of a lambda function. When referencing a directory, the files stays in-place when being zipped. However, when referencing a single file, a temporary directory is created and the file is copied there before the directory is zipped.

Before, the file stats were not copied, leading to new modification times every time cloudformation package were run referencing a single file.

This change also copies the file stats, ensuring the zip file gets the same checksum and filename as previous runs, and avoids an necessary upload and change set of the CFN stack, unless the source file gets a new modification time or changes content.

Partly solves #3131. However, the source file still needs to have the same modification time as previous to avoid a new checksum. In a CI-environment the file will probably get a different modification time for every checkout. This can be mitigated by forcing a modification timestamp on the source file, e.g. by touch -t '200001010000' file.py before running cloudformation package.

I'm a bit unsure about the tests I've added. Don't like adding time.sleep(), but not sure how to test this otherwise.

@codecov-io
Copy link

codecov-io commented Jul 9, 2018

Codecov Report

Merging #3436 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3436      +/-   ##
===========================================
+ Coverage    94.41%   94.41%   +<.01%     
===========================================
  Files          169      169              
  Lines        13252    13253       +1     
===========================================
+ Hits         12512    12513       +1     
  Misses         740      740
Impacted Files Coverage Δ
...customizations/cloudformation/artifact_exporter.py 97.44% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0fad3f...e57834a. Read the comment docs.

@stealthycoin
Copy link
Contributor

Thanks for the PR! @sanathkr can you review this?

@ashwgupt
Copy link

Hi Guys,

Is there any timelines on when will this fix be going in?

@codecov-commenter
Copy link

Codecov Report

Merging #3436 into develop will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3436   +/-   ##
========================================
  Coverage    94.41%   94.41%           
========================================
  Files          169      169           
  Lines        13252    13253    +1     
========================================
+ Hits         12512    12513    +1     
  Misses         740      740           
Impacted Files Coverage Δ
...customizations/cloudformation/artifact_exporter.py 97.44% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8730300...e57834a. Read the comment docs.

thoward-godaddy pushed a commit to thoward-godaddy/aws-cli that referenced this pull request Feb 12, 2022
@RyanFitzSimmonsAK
Copy link
Contributor

Hi @henrist, thanks for creating this PR and for your patience. Could you please rebase this PR against the latest branch for the team’s further consideration going forward?

@RyanFitzSimmonsAK
Copy link
Contributor

Going to close this PR for the time being. If someone later decides they want to pick this up, rebase it and it will be reopened for consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants