-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Revert "pkg/asset: Introduce Load() into the Asset interface that loads assets (from disk)" #494
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
Conversation
…ds assets (from disk)"
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deads2k If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Can we adjust to only load files that match paths from the graph? |
|
Yifans PR preloads the dir on startup and then serves the asset from internal cache. |
|
Would it be hard to populate on demand? |
|
@wking is working on an alternate fix. /hold |
|
Ok, I've pushed #497 replacing the early |
|
.
On-demand is clean, but it will be much slower. Can't we make Walk more robust? No need to error.. just put only valid values in the cache. |
How many files are we loading? Should be a handful of explicit files and a few globs. I can benchmark if you like, but I'm guessing this will be performant enough. |
|
@deads2k your git fu is bad and you should feel bad :) |
Reverts #374
sorry @stevekuznetsov I don't know how to create this pull locally.
Multiple reports of: I run with libvirt and after this pull merged I see
resetting to bf0fa6d gives me a working binary
@sallyom @djzager @yifan-gu @wking @abhinavdahiya
This is one option. If we know how to push through, I'm fine with that too.