feat: Filter the directories/files hashed when using commands with patterns, docs and examples fixes#720
Conversation
4776bb9 to
784f889
Compare
antonbabenko
left a comment
There was a problem hiding this comment.
Looks pretty good! Thanks for tackling this issue.
Could you please explain how to test it? Compare this one to the current version of the module and explain what is expected/fixed (changed or not changed).
|
Thank you for the review and feedback! The fix can be exemplified with this code: and the following steps. With the current version of the module: We can see that ignore.txt is not included in the archive, but changing its content makes the module want to recreate the archive, with a different name, due to the changed hash. With the proposed set of changes: We can see that after changing the content of ignore.txt, the plan shows no changes. I would like to ask what do you think about the On the other hand, for the goal of hashing the content of |
This ensures that when sharing code between different platforms/environments, this will not generate unneeded changes. Base64 results in different values for the same data across different platforms. Ties in nicely with terraform-aws-modules#720.
|
This PR has been automatically marked as stale because it has been open 30 days |
784f889 to
8be2425
Compare
|
I have updated the changes to apply to version 8.2.0. @antonbabenko could you consider this again, please? Thank you! |
antonbabenko
left a comment
There was a problem hiding this comment.
Works great, but hash_extra_paths or hash_internal is not needed. Please simplify, and I will merge it right away.
PS: Thank you for this important fix!
package.tf
Outdated
| runtime = var.runtime | ||
| source_path = try(tostring(var.source_path), jsonencode(var.source_path)) | ||
| hash_extra = var.hash_extra | ||
| hash_extra_paths = jsonencode([]) |
There was a problem hiding this comment.
I think hash_extra_paths is not needed anymore, since there is already hash_internal. Let's drop one and we are good to go with this PR.
Previously, the following paths were normalized: the working directory for commands and the path elements when source_path is a list. With this change, the following paths are normalized too: plain paths given as source_path strings, path values of npm_requirements and pip_requirements.
See PR terraform-aws-modules#66 TODO: remove the code and logic for hash_extra_paths?
…ecated attributes with version 6.x of AWS provider
048e18a to
a45412c
Compare
a45412c to
eb54586
Compare
|
Sorry for the delay. I have removed the code for |
|
@flora-five Please fix the failing tests (I know this is not related to the PR), and we are good to go! |
bddb8d7 to
db8bbf7
Compare
|
The additional changes update to the latest version of ruff, which fixes one failure. The other change that makes the tests pass is to remove python 3.8 and 3.9 from the test matrix, because they are EOL. Other changes add python 3.13 and 3.14 to the test matrix, update to the latest versions of pytest and poetry, fix repeated need to apply |
## [8.5.0](v8.4.0...v8.5.0) (2026-02-02) ### Features * Filter the directories/files hashed when using commands with patterns, docs and examples fixes ([#720](#720)) ([daa5dfc](daa5dfc))
|
This PR is included in version 8.5.0 🎉 |
Description
This set of changes does the following:
os.path.normpathto normalize other paths used as input to the prepare steppackage.pyjsencode, when passed from Terraform to pythonMotivation and Context
When commands and patterns are used together, the patterns filter is applied when building the archive, but not during the prepare step. Files that are not included in the archive due to the filter are still hashed. If some of those files are generated, with their content depending on the build environment, then, even if the built archives are the same across different build environments, the calculated hashes are not the same and the module wants to create again the archive and redeploy. Making the hashing step take into account the patterns filter fixes #672.
Hashing again the content of
package.pyrestores the logic that was necessary to disable in #66 to fix #63. This time only the content ofpackage.pyis hashed, without its path.The change to normalize other paths used as input with
os.path.normpathis for consistency with existing normalization on some input paths.Breaking Changes
None, but all the hashes will change due to inclusion of
package.pyinto the hashing.How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectspre-commit run -aon my pull request