ignore excluded paths when transferring files #163
Conversation
Glyphack
changed the title
|
Hey @lbajolet-hashicorp sorry for directly pinging, but this PR was open for a while I wanted to see if I can get any help from you on this. I have tested the this on the aws plugin and it works I'm not sure where can we add the acceptance for this as there's no acceptance test on the repo. Thanks for your time. |
There was a problem hiding this comment.
Hi @Glyphack,
Sorry I took so long to take a look at this PR, this slipped my mind, apologies again.
Taking a look at the code, the approach looks OK to me, about the excludes attribute that you added to the file provisioner, I believe this would be better as part of the communicator config directly, so that it will work automagically for all the plugins that update their SDK dependency.
In addition to this, and I know that in this repo we don't have that setup since it relies on plugins to test, but I think it would be great to have some acceptance tests to make sure the excludes option works in all cases.
While working on #180, I wrote a series of acceptance tests for the scp and sftp copies, it's not public yet, but if you're working on this code as well, I can maybe give you access to this repository so you can base on that to work on acceptance tests for this feature.
This requires a bit of fiddling since both Packer and the qemu plugins need to be compiled against an up-to-date sdk for the tests to work, but if necessary we can help you on this.
Let me know if that would interest you, otherwise I can also write some tests and run them against this PR.
Thanks again for the proposal, it's an interesting feature which I'm sure will be helpful to many users.
| testCases := []struct { | ||
| Exclude []string | ||
| Path string | ||
| Is bool |
There was a problem hiding this comment.
In terms of clarity, this bool seems to be used to check that we did exclude something, could you rename it to make it clearer what it's used for?
That's personal preference, but I found that constructions like ExpectExclude works well at communicating the intent, but feel free to give it your own take on that!
|
Just wanted to let you know that I have this in my radar, and will get back to it next week latest. In the meanwhile I can get some help on how to continue with updating [winRM] (hashicorp/packer#12280 (comment)) uploader. |
|
Thanks for the comments @lbajolet-hashicorp, I've applied them the only one which I could not address was winRM for the reason I mentioned in previous comment. Regarding tests, I'm interested in learning it from you. Let me know when you have time to share the code or look into it together. |
|
any update on this? would be really nice to have |
This PR implements the exclude file feature related to this PR in packer repo.
While uploading directories each object is checked against exclude list, and if it's an exact match that object(file, symlink, directory) is skipped. The pattern matching is done using filepath.Match and I'm open to changing it to another syntax like regex.
I have wrote unit tests for it but for testing it completely I have used it on aws plugin to make sure it does not upload the file.
I'd be happy if you can provide some initial feedback on the approach to resolve the issue.