Fix partial caching ignore repeated items issue #35145
Merged
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
st0012
force-pushed
the
fix-35114
branch
2 times, most recently
from
878a36b
to
0ea0bc6
Compare
|
@amatsuda are you willing to give this a quick scan if you have time? I'd really appreciate it 🙇 |
rafaelfranca
reviewed
Apr 3, 2019
| @@ -56,8 +60,11 @@ def collection_by_cache_keys(view, template) | |||
|
|
|||
| digest_path = view.digest_path_from_template(template) | |||
|
|
|||
| @ordered_keys = [] | |||
There was a problem hiding this comment.
Instead of building this instance variable here and using in other methods we should return it and pass around.
|
@rafaelfranca updated |
rafaelfranca
requested changes
Apr 3, 2019
This is because we only use hash to maintain the result. So when the key are the same, the result would be skipped. The solution is to maintain an array for tracking every item's position to restructure the result.
|
@rafaelfranca done |
krauselukas
added a commit
to krauselukas/open-build-service
that referenced
this pull request
Apr 15, 2020
In certain cases, the md5 and mtime value of a file is the same for multiple files. Only the file name differs in those cases. This needs to be considered, in order to have uniq data in the cache. In the past this lead to not show certain files in the webui due to this rails/rails#35145 Since this got fixed, it now does the opposite (showing duplicated entries), which is more obvious. Since osc shows the correct files, and missing files are less obvious than duplicates, this probably took a while to be reported as an issue. Fixes openSUSE#9352 Co-authored-by: David Kang <dkang@suse.com> Co-authored-by: Eduardo Navarro <enavarro@suse.com>
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.
Summary
This is for #35114
Partial caching doesn't return the right collection when some of the items are repeated.
Let's say we have this template:
And a
cached_setcollection:On current master, the rendered result will be
The duplicated items are ignored.
This is because we only use hash to maintain the result. So when the key are the same, the result would be skipped.