Update caching mechanism to allow for concurrent genpolicy runs #113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SethHollandsworth
force-pushed
the
feature/update_caching
branch
from
d165d78 to
090e5f8
Compare
|
Thanks @SethHollandsworth! This change seems very promising at a quick look, but please rebase on the latest branch before we review it more thoroughly. |
SethHollandsworth
force-pushed
the
feature/update_caching
branch
from
090e5f8 to
88e5257
Compare
|
rebased and ready to review, thanks! |
SethHollandsworth
force-pushed
the
feature/update_caching
branch
from
336be43 to
d7c6cfb
Compare
|
Thanks again for the help, Seth! I think we should try to detect if another app/instance is using the cache, and switch to non-cached mode if that's the case. Otherwise we are still racing while modifying the cache from two or more instances. |
SethHollandsworth
force-pushed
the
feature/update_caching
branch
2 times, most recently
from
8009d31 to
c698e5a
Compare
|
This should be merged after #117 . I will rebase and ensure building on Windows works at that point. Thanks for being flexible! |
SethHollandsworth
force-pushed
the
feature/update_caching
branch
2 times, most recently
from
6fdd439 to
b97fd54
Compare
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
danmihai1
reviewed
Dec 6, 2023
SethHollandsworth
force-pushed
the
feature/update_caching
branch
from
b97fd54 to
3de20d4
Compare
danmihai1
requested changes
Dec 7, 2023
Previously the tool would use the layers_cache folder for all instances and hence delete the cache when it was done, interfereing with other instances. This change makes it so that each instance of the tool will have its own temp folder to use. Co-authored-by: Khalil Sayid <[email protected]> Signed-off-by: Seth Hollandsworth <[email protected]>
SethHollandsworth
force-pushed
the
feature/update_caching
branch
from
3de20d4 to
565df4a
Compare
ms-mahuber
approved these changes
Dec 15, 2023
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is not a requirement of the tool itself but of the
katapolicygenpython wrapper used in confcom for writing tests that run in parallel. Previously thelayers_cachefolder would be concurrently written to and deleted depending on the settings of the tests, resulting in errors.This PR proposes using temp folders instead of
layers_cacheand keeping the layer hashes in a centralpolicy.jsonfile that doesn't get deleted after running the tool instead of the.verityfiles nested inlayers_cache. Because of the use of temporary folders, the cleanup process is a little nicer too.Let me know if anything should be changed, thanks!