This repository has been archived by the owner on Jun 2, 2022. It is now read-only.
Explore volumes incrementally, sort API list response, make history entry more readable #330
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
plugin.CachedList returns a map, which removes any ordering on the list results from an individual plugin. Remove ignored sorting from volume.List and instead sort when constructing the API response. FUSE appears to handle sorting entries for us when using `ls`. Signed-off-by: Michael Smith <[email protected]>
ekinanp
reviewed
Jun 14, 2019
| @@ -60,6 +61,8 @@ var listHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes | |||
| apiEntry.Path = path + "/" + apiEntry.CName | |||
| result = append(result, apiEntry) | |||
| } | |||
| // Sort entries so they have a deterministic order. | |||
| sort.Slice(result, func(i, j int) bool { return result[i].Name < result[j].Name }) | |||
There was a problem hiding this comment.
Sucks that we have to iterate again. Guessing there's no easy OrderedMap alternate?
There was a problem hiding this comment.
I don't think it's too bad, O(n log n) either way. There are several options:
There was a problem hiding this comment.
Yeah, not worth it. Like you said, O(n log n) isn't that bad.
Parses the logfmt entries returned by the API and emits them in a more
readable format, e.g.
```
Jun 13 15:44:04.299 Exec [find / -mindepth 1 -maxdepth 5 -exec stat -c %s %X %Y %Z %f %n {} +] on blissful_gould
Jun 13 15:44:04.433 stdout: 4096 1559079604 1557434981 1559079604 41ed /lib
2597536 1552660099 1552660099 1559079604 81ed /lib/libcrypto.so.1.1
```
Tests were incorrectly relying on uninitialized fields in a map returning nil. That wasn't the case. Use Contains to correctly test the contents of a map. Signed-off-by: Michael Smith <[email protected]>
MikaelSmith
force-pushed
the
volume-improvements
branch
from
dfcca03 to
476b88f
Compare
The volume package will now search to the specified depth, then if that level is listed it will incrementally search the subtree requested. This provides improved speed for individual requests while batching listing for slower transports. Signed-off-by: Michael Smith <[email protected]>
MikaelSmith
added
the
do not merge
MikaelSmith
force-pushed
the
volume-improvements
branch
from
476b88f to
d211318
Compare
MikaelSmith
removed
the
do not merge
Refactor volume internals to remove the path argument from volume.List,
which always needed to be set to an empty string ("") when called
externally.
Signed-off-by: Michael Smith <[email protected]>
MikaelSmith
force-pushed
the
volume-improvements
branch
from
d211318 to
74e74f9
Compare
ekinanp
approved these changes
Jun 14, 2019
MikaelSmith
added a commit
to MikaelSmith/wash
that referenced
this pull request
Jun 15, 2019
PR puppetlabs-toy-chest#330 introduced a regression where, when the cache expired, listing a volume could repopulate the cache with different data and break navigation. Fix so that VolumeList calls for a particular cache key always uses the same path. Signed-off-by: Michael Smith <[email protected]>
MikaelSmith
added a commit
to MikaelSmith/wash
that referenced
this pull request
Jun 15, 2019
PR puppetlabs-toy-chest#330 introduced a regression where, when the cache expired, listing a volume could repopulate the cache with different data and break navigation. Fix so that VolumeList calls for a particular cache key always uses the same path. Signed-off-by: Michael Smith <[email protected]>
MikaelSmith
added a commit
to MikaelSmith/wash
that referenced
this pull request
Jun 17, 2019
PR puppetlabs-toy-chest#330 introduced a regression where, when the cache expired, listing a volume could repopulate the cache with different data and break navigation. Fix so that VolumeList calls for a particular cache key always uses the same path. Signed-off-by: Michael Smith <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Resolves #290.