Avoid spurious downloads during dependency management #1124
Conversation
stevendanna
force-pushed
the
ssd/stahp-using-my-bandwidth
branch
from
953b6c2
to
c67d1c1
Compare
| opts[:backend] = @backend | ||
| if !@dependencies.nil? | ||
| opts[:dependencies] = Inspec::DependencySet.from_array(@dependencies, @cwd, @cache, @backend) | ||
| end | ||
| @profile ||= Inspec::Profile.for_target(opts, opts) | ||
| @profile = Inspec::Profile.for_fetcher(fetcher, opts) |
There was a problem hiding this comment.
I was thinking about this for a while, since it now deviated from our CLI code. We are using a separate mechanism for inspec json now.
There was a problem hiding this comment.
Can you rephrase this comment? I'm don't understand what you are trying to indicate.
There was a problem hiding this comment.
Discussion: We are going to leave for_fetcher() but change it so that for_target calls for_fetcher so that it is hard to introduce drifting behavior between the two, this will require the fetcher to expose the original target via an attr_accessor.
There was a problem hiding this comment.
This is done now.
stevendanna
force-pushed
the
ssd/stahp-using-my-bandwidth
branch
from
c67d1c1
to
456922d
Compare
| @@ -72,9 +35,14 @@ def self.for_path(path, opts) | |||
| new(reader, opts) | |||
| end | |||
|
|
|||
| def self.for_fetcher(fetcher, opts) | |||
| path, writable = fetcher.fetch | |||
| for_path(path, opts.merge(target: fetcher.target, writable: writable)) | |||
There was a problem hiding this comment.
wouldn't it be better to attach the fetcher to the profile and just read the target then from the fetcher?
There was a problem hiding this comment.
@chris-rock My plan was to do soemthing like that in a follow up PR since I think that is ultimately what we need to do to remove the Requirement class.
There was a problem hiding this comment.
Alright, lets go for that approach
Before, a URL based source might be downloaded multiple times during the dependency fetching and lockfile creation. This commit tries to avoid this by: 1) Memoizing data about the archive to avoid re-fetching the archive 2) Adding a CachedFetcher wrapper around the fetcher class to help ensure that callers always consult the cache before fetching. Signed-off-by: Steven Danna <steve@chef.io>
chris-rock
force-pushed
the
ssd/stahp-using-my-bandwidth
branch
from
456922d
to
50b27c4
Compare
|
Thanks @stevendanna for this improvement. |
Before, a URL based source might be downloaded multiple times during the
dependency fetching and lockfile creation. This commit tries to avoid
this by:
Memoizing data about the archive to avoid re-fetching the archive
Adding a CachedFetcher wrapper around the fetcher class to help
ensure that callers always consult the cache before fetching.
Signed-off-by: Steven Danna steve@chef.io