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

Problem with verifier command caching #198

Open
opyh opened this issue Nov 28, 2014 · 7 comments
Open

Problem with verifier command caching #198

opyh opened this issue Nov 28, 2014 · 7 comments

Comments

@opyh
Copy link

opyh commented Nov 28, 2014

Verifiers seem to cache their commands in the prepare method, which is an issue for me:

I want to write a package that downloads files generated on server to my local machine, using the matches_local verifier to see if the md5 sums match. To make the matcher work, I had to touch the files first so it doesn't fail because they don't exist.

While the package is run, the file is downloaded, but this changes the local file, changing its md5 sum too — so after the download, instead of the changed file, the empty file is compared to the server's version.

I could write my own verifier, maybe overriding the commands method like this:

def commands
    @prepared = false
    prepare
    @commands
end

…but this looks a bit hacky.

Is the caching a necessary optimization? Would it be sensible to take it out to not make it dependent on the state before a package is installed? Or is my approach nonsense and there's a better approach?

@joshgoebel
Copy link
Contributor

Please provide your sprinkle itself or a relevant excerpt.

@joshgoebel
Copy link
Contributor

Verifiers are build from a block... we have NO way of knowing what that block does but MOST peoples assumption is the block is only run once when the verifier is parsed. So the commands have to be cached after the block is run once... running the block again could be adverse or cause unexpected behavior.

Not that it can't work, but I don't think you're really doing something Sprinkle was designed for.

@opyh
Copy link
Author

opyh commented Dec 1, 2014

I did not assume this (I think a verifier should not create its own side effects, but should be able to cope with side effects caused by the package installation) but you are probably right about most people. What I want to achieve is an edge case.

The following package fails if the verifier block is only run once because the MD5 changes when the package is downloaded. Yes, it's hacky. ;)

package :download_file_via_scp do
  local, remote, host = opts[:local], opts[:remote], opts[:host]
  if local && remote && host
    runner '' do
      pre :install do
        cmd = "scp root@#{host}:#{remote} #{local}"
        puts cmd
        puts `#{cmd}`
        []
      end
    end
    verify do
      FileUtils.touch local
      matches_local local, remote
    end
  end
end

Will try to find a different solution.

Probably something like Commands::Transfer for downloading files would be clean, but some work to implement if it's supposed to work with all agents you support…

@joshgoebel
Copy link
Contributor

I think a verifier should not create its own side effects

I'm not sure what that means. The verifier code is run at parse-time so any errors can be detected. It's likely be easy to hack your own verifier class to run the code at run-time (your first post kind of had this idea), but then you wouldn't find out about any errors in your code until the middle of your script execution, which could be bad.

I'd probably write a package type that did the download and confirm in the package itself... honestly verification of file transfers sounds like something the file transfer should be dealing with, not verifiers... Checking the sums later also sounds a bit paranoid... do you not trust scp? If it exits without an error the files should have been transferred successfully without need for another check.

Maybe doing something like running rsync twice (with checksums) would be a far simpler solution.

@joshgoebel
Copy link
Contributor

Really this is an issue with matches_local defying the assumption that static commands are what should be generated. It's using Ruby to generate the md5... I knew there was a reason that rubbed me the wrong way when I was proposed.

@joshgoebel
Copy link
Contributor

Here is the commit I'm referring to #65

@joshgoebel
Copy link
Contributor

@opyh Hack it to turn off the caching and see if that works for you. Maybe we could step back from the idea that the verification block is only truly parsed once and just let it happen each time... although I'm wondering if there are people taking advantage of the existing system that that would break things of.

It gets weird to start inserting block and procs inside verify to tell it what should run real-time vs parse-time... when perhaps it wouldn't hurt if the entire thing just ran in real-time.

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

No branches or pull requests

2 participants