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

Caching does not work as expected #45

Open
heyimalex opened this issue Nov 29, 2023 · 0 comments · May be fixed by #46
Open

Caching does not work as expected #45

heyimalex opened this issue Nov 29, 2023 · 0 comments · May be fixed by #46

Comments

@heyimalex
Copy link

From the configuration file:

By default c.cache is set to false, which means an application always parses a manifest.json. In development, you should set cache false usually. Instead, setting it true which caches the manifest in memory is recommended basically.

However, all of the helpers call get_manifest_by_key which calls Minipack.configuration.manifests. Looking at the source for manifests:

      def manifests
        raise Error, 'Calling #manifests is only allowed from a root' unless root?

        repo = ManifestRepository.new
        #  Determine if a single manifest mode or multiple manifests(multiple site) mode
        targets =  @children.empty? ? [self] : @children.values
        targets.each do |config|
          # Skip sites that a manifest file is not configured
          next if config.manifest.nil?

          repo.add(config.id, config.manifest, cache: config.cache)
        end
        repo
      end

A new repo is created on every call, which means that new manifest is created on every call, which means that we re-read and parse the manifest file on every call. You can verify this in an irb window. Notice that cache is true but the addresses of the returned manifests are different.

> Minipack.configuration.manifests.default
=> #<Minipack::Manifest:0x00007faea98fc1e0 @path="/opt/platform/manifest.json", @cache=true>
> Minipack.configuration.manifests.default
=> #<Minipack::Manifest:0x00007faea0375fb8 @path="/opt/platform/manifest.json", @cache=true>
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 a pull request may close this issue.

1 participant