File filtering improvements #332
Conversation
anderseknert
force-pushed
the
efficient-filter
branch
from
e7ff65c
to
410012a
Compare
|
Heh, pretty much all of my findings from this experiment summarized here: https://benhoyt.com/writings/go-readdir/ |
anderseknert
force-pushed
the
efficient-filter
branch
from
410012a
to
372d742
Compare
|
Insipired by that blog, I rewrote the implementation using @srenatus please take another look and let me know what you think :) Using |
anderseknert
force-pushed
the
efficient-filter
branch
from
372d742
to
1d7635c
Compare
pkg/config/filter.go
Outdated
| // to work with FileInfo objects, but will rather work directly with the files as | ||
| // string paths. Finally, this function differs from OPA in that we never will | ||
| // traverse down into .git or .idea directories, as that's always a waste of time. | ||
| func walkPaths(paths []string, filter DirEntryFilter) []error { |
There was a problem hiding this comment.
[nit] It doesn't matter, but golang can join errors now: https://pkg.go.dev/errors#Join
There was a problem hiding this comment.
Oh, that's interesting! Perhaps we could skip that whole errorsToString function entirely then? I'll play around with it.
pkg/config/filter.go
Outdated
| buf := make([]string, len(e)) | ||
| for i := range buf { | ||
| buf[i] = e[i].Error() | ||
| } | ||
|
|
||
| return fmt.Sprintf("%v errors occurred during loading:\n", len(e)) + strings.Join(buf, "\n") |
There was a problem hiding this comment.
[nit] Inconsequential, but I've played with it so I might as well post the comment 🙃
| buf := make([]string, len(e)) | |
| for i := range buf { | |
| buf[i] = e[i].Error() | |
| } | |
| return fmt.Sprintf("%v errors occurred during loading:\n", len(e)) + strings.Join(buf, "\n") | |
| buf := make([]string, len(e)+1) | |
| buf[0] = fmt.Sprintf("%v errors occurred during loading:", len(e)) | |
| for i := range buf { | |
| buf[i+1] = e[i].Error() | |
| } | |
| return strings.Join(buf, "\n") |
pkg/config/filter.go
Outdated
| ) | ||
|
|
||
| func FilterIgnoredPaths(paths, ignore []string, checkFileExists bool) ([]string, error) { | ||
| policyPaths := paths | ||
| type DirEntryFilter = func(path string, info os.DirEntry, err error) error |
There was a problem hiding this comment.
[nit] DirEntryFilter doesn't need to be exported. I wonder if it even warrants its own type, but it won't hurt.
There was a problem hiding this comment.
You're right, not at this point. I guess we really should expose something like lint.WithFilter(...).Lint(ctx) or else users of the Go API wouldn't be able to tell us what they consider to be Rego files. Not important right now, so I'll un-export this with a note.
anderseknert
force-pushed
the
efficient-filter
branch
from
1d7635c
to
b00e24c
Compare
* Ignore .git and .idea directories * Use more efficient algorithm for filtering than the one provided by OPA Fixes #330 Signed-off-by: Anders Eknert <anders@styra.com>
anderseknert
force-pushed
the
efficient-filter
branch
from
b00e24c
to
8e05735
Compare
| return filterPaths(paths, ignore) | ||
| } | ||
|
|
||
| func walkPaths(paths []string, filter func(path string, info os.DirEntry, err error) error) error { |
There was a problem hiding this comment.
LOL, errlang strikes again! But you're right, that type was probably superfluous.
Fixes #330