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

Allow licensed apps to share caches #263

Merged
merged 4 commits into from
May 15, 2020
Merged

Allow licensed apps to share caches #263

merged 4 commits into from
May 15, 2020

Conversation

jonabc
Copy link
Contributor

@jonabc jonabc commented May 15, 2020

This adds in an option for users to configure apps that share a cache path, and works both for explicitly declared apps as well as apps defined by glob patterns.

Source paths given with glob patterns need special handling to not append the app folder name to an explicitly specified cache path. In this scenario users can add a shared_config: true configuration value to the same configuration level as the glob source path.

From the docs update:

Sharing caches between apps

Dependency caches can be shared between apps by setting the same cache path on each app.

apps:
  - source_path: "path/to/app1"
    cache_path: ".licenses/apps"
  - source_path: "path/to/app2"
    cache_path: ".licenses/apps"

When using a source path with a glob pattern, the apps created from the glob pattern can share a dependency by setting an explicit cache path and setting shared_cache to true.

source_path: "path/to/apps/*"
cache_path: ".licenses/apps"
shared_cache: true

/cc @zarenner this should allow you to cache all dependencies to the same folder. Does this seem like a viable solution?

@zarenner
Copy link
Contributor

Tested it against my repo, looks good! Thanks!

@zarenner
Copy link
Contributor

Minor nit:
I'm guessing this might have been intentional, but I would have expected the syntax

cache_path: ".licenses"
shared_cache: true
apps:
  - source_path:  foo
  - source_path:  bar

to share .licenses and it doesn't (have to individually specify cache_path for each app)?

@jonabc
Copy link
Contributor Author

jonabc commented May 15, 2020

I'm guessing this might have been intentional, but I would have expected the syntax

🤔 it's intentional that the shared_cache value isn't inherited for cases where the cache_path is set at the same level as an app. So in the following scenario the setting wouldn't apply to cache_path: .licenses/foo.

shared_cache: true
apps:
  - source_path: foo/cmd/*
     cache_path: .licenses/foo
  - source_path: bar

Sharing a cache is potentially dangerous and should be a reasoned decision based on user needs, so the intention is that it only applies to cache_paths defined at the same level. I think what you found is a bug - I'll make that update, thanks!

@jonabc jonabc merged commit 2dde0f8 into master May 15, 2020
@jonabc jonabc deleted the shared_caches branch May 15, 2020 17:38
@jonabc jonabc mentioned this pull request May 15, 2020
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 this pull request may close these issues.

2 participants