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

export checksum code #188

Merged
merged 2 commits into from
Jun 27, 2019
Merged

export checksum code #188

merged 2 commits into from
Jun 27, 2019

Conversation

SwampDragons
Copy link
Contributor

@SwampDragons SwampDragons commented Jun 27, 2019

Adrien wrote code to allow the go-getter to handle checksums from files, so that Packer didn't need to handle file checksums in its own internal logic. But we missed a "fun" edge case where we were using the checksum information pulled from that file to decide whether or not to upload a file to a vmware esxi machine.

Packer needs to be able to access the actual checksums contained within the file. The only way to do that apart from re-implementing all of the file parsing code from go-getter for this one edge case is to allow the client to call the checksum extraction directly.

I don't think making this method exported is a particularly bad thing, but I'd appreciate a sanity check... maybe from @vancluever who reviewed the original checksum file code.

FWIW I've tested this change within Packer and it does solve my issue.

@vancluever
Copy link
Contributor

Thanks for referencing the PR where you're using this @SwampDragons! It helps to see how it looks in action.

I don't see an issue in exporting this either - the only thing that I'd like to see is comments adjusted to reflect the new export as well, in addition to fileChecksum being exported as well.

@SwampDragons
Copy link
Contributor Author

Awesome. I can do those.

Also, re: test failures, I've opened #189 to fix this bitbucket failure.

@vancluever
Copy link
Contributor

Beauty! Thanks for taking care of that too!

Copy link
Contributor

@vancluever vancluever left a comment

Choose a reason for hiding this comment

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

I think there might be a couple of helpers that could be exported at a later time, but I don't want to rabbit-hole this so this is fine 🙂

@SwampDragons SwampDragons force-pushed the export_checksum_file_code branch from 7ffa167 to 025d982 Compare June 27, 2019 22:21
@SwampDragons SwampDragons merged commit da0323b into master Jun 27, 2019
@SwampDragons SwampDragons deleted the export_checksum_file_code branch June 27, 2019 22:31
@azr azr mentioned this pull request Feb 4, 2020
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