Support for exclude include #212
Conversation
pkg/modfilter/modfilter.go
Outdated
| } | ||
|
|
||
| func getConfigLines() ([]string, error) { | ||
| configName := configurationFileName |
There was a problem hiding this comment.
this i will update with new config package once it's in
Codecov Report
@@ Coverage Diff @@
## master #212 +/- ##
==========================================
- Coverage 45.8% 45.34% -0.47%
==========================================
Files 84 88 +4
Lines 2039 2245 +206
==========================================
+ Hits 934 1018 +84
- Misses 990 1111 +121
- Partials 115 116 +1
Continue to review full report at Codecov.
|
pkg/modfilter/modfilter.go
Outdated
| ) | ||
|
|
||
| // ModFilter is a filter of modules | ||
| type ModFilter struct { |
There was a problem hiding this comment.
can you name this Filter? that way, the type will read mod.Filter when it's used - instead of the redundant mod.ModFilter
pkg/modfilter/modfilter.go
Outdated
| // - | ||
| // + github.com/a | ||
| // will exclude all items from communication except github.com/a | ||
| func NewModFilter() *ModFilter { |
There was a problem hiding this comment.
can you name this NewFilter()? then the call will be mod.NewFilter() which will be less redundant
There was a problem hiding this comment.
also, do you mind adding a note here that this struct is not concurrency safe? not sure if that matters right now, but it'll be nice to have that later on at least as a reminder 😄
| return func(args worker.Args) (err error) { | ||
| module, version, err := parseArgs(args) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if !mf.ShouldProcess(module) { | ||
| return fmt.Errorf("Module %s is excluded", module) |
There was a problem hiding this comment.
can you make this an error type so we can check for it in other places?
| "github.com/gomods/athens/pkg/paths" | ||
| "github.com/gomods/athens/pkg/storage" | ||
| ) | ||
|
|
||
| func cacheMissHandler(next buffalo.Handler, w worker.Worker) buffalo.Handler { | ||
| func cacheMissHandler(next buffalo.Handler, w worker.Worker, mf *modfilter.ModFilter) buffalo.Handler { | ||
| return func(c buffalo.Context) error { | ||
| nextErr := next(c) |
There was a problem hiding this comment.
wouldn't we want to check if the module should not be processed before we go to the next step?
| // If no filename is specified it fallbacks to 'filter.conf' | ||
| func IncludeExcludeFileName() string { | ||
| return envy.Get("ATHENS_FILTER_FILENAME", defaultConfigurationFileName) | ||
| } |
ghost
mentioned this pull request
I was thinking about two options here, first one with strong ordering and first rule matching wins.
The second one a bit complicated to implement but easier to use. No ordering reqs for user to take into account.
I went with second as it is more fun to write and the user is happier.
Based on filter.conf file proxy decides which modules to process and which not.
If there is '-' rule for module or one of its parents it will not process the module and won't report cache miss to Olympus either.
Example
this will process:
wont process
Second example
notice
-with nothing after it - it means global excludeThis won't process everything except a/b/c and its children (a/b/c/d)
So the way it is decided for
a/b/c/dis to check rule fora/b/c/dand then iterate upwards until non-default rule is found (rules: Default, Exclude, Include)Fixes #96