Rsync option #254
Rsync option #254
Conversation
|
I'll need to add some tests and documentation as well. |
|
Would your problem be solved if instead of rmtree the destdir it deletes The code looks good, would definitely need some docs and tests, it could even be the default and remove all the custom cp logic, permissions,... |
|
Cool cool cool. I would even vote to make this default, because when you create a puppet master's module-directory with |
|
The only reason I would hesitate about making this the default behavior is that we would not be able to control the dependency on @carlossg I haven't tested the @njam thanks for chiming in. I was wondering if I was the only one seeing this behavior! I'll add some tests and docs today. |
|
I just pushed a cucumber scenario that captures this, but it feels kind of hacky. Could I get some input on the |
|
Looks good to me, doesn't the same problem happen when running librarian-puppet install after the first time? |
|
It does, I'll add a couple more tests and docs soon. I've been swamped with PuppetConf stuff and I haven't had a chance to touch this for the past few days. |
|
What do you guys think? Is this code ready to go? |
|
Actually, I've found one issue with this. I'm going to add another test case and fix it. |
|
Can you squash the commits into one for merging? |
Need to use the parent of dest for rsync Reverting gemfile Adding a test. Going to check with maintainers to see if this is a valid testing strategy. Adding ctime to the mix to make sure that the file isn't just assigned the same inode Adding tests for the rsync option using the install command Only compare ctime if inodes match Adding test case to make sure update works properly with git
|
Squashed :) |
|
Thanks! |
|
Seems there are some timing issues ? |
|
Strange. I'll dig into it! |
|
Do you want to revert for now? |
|
No if it doesn't take too long to fix. I've tried with 7506ced but seems it didn't fix it |
|
I think it may be the logic in my test. Hopefully I'll have a commit for you in a few hours. |
|
Sorry, I've had a heck of a time replicating this. |
|
seems it only happens in the 1.x branch |
We're using librarian-puppet in a large-scale environment with lots of change and we've run into issues with librarian-puppet's rather liberal convergence strategy. Essentially we're running into an issue where a
librarian-puppet updatecommand will result in a Puppet master process losing it's CWD because of theinstall_path.rmtreecall. This results in a catalog compilation failure. Theinstall_path.rmtreemethod gets called regardless of whether there are changes to the module ininstall_path. This PR allows for the option to use rsync to converge the directories, which is ultimately a much more conservative convergence strategy. This may not be the most idiomatic implementation. I'm open to suggestions on how to improve it.