bundle: Implement a DirectoryLoader for fs.FS #3493
Conversation
simongottschlag
force-pushed
the
feature/bundle/NewFSLoader
branch
2 times, most recently
from
5a67295
to
23ff632
Compare
|
@srenatus thank you so much for the review! Code is much cleaner now. The only question left is in regards to the concurrency and if that's something that should be addressed in this PR or not - I don't think we're introducing new behaviour or issues in comparison to |
simongottschlag
force-pushed
the
feature/bundle/NewFSLoader
branch
2 times, most recently
from
5cf67c9
to
39f8315
Compare
|
@srenatus added mutex and swapped errors.Wrap to fmt.Errorf :) |
simongottschlag
force-pushed
the
feature/bundle/NewFSLoader
branch
from
6fe835b
to
866ac9c
Compare
This comment has been minimized.
This comment has been minimized.
simongottschlag
force-pushed
the
feature/bundle/NewFSLoader
branch
from
f53248d
to
9c76a06
Compare
To make it possible to create a DirectoryLoader based on the fs.FS interface. This interface requires go version 1.16 or above. Fixes: open-policy-agent#3489 Signed-off-by: Simon Gottschlag <simon.gottschlag@xenit.se>
simongottschlag
force-pushed
the
feature/bundle/NewFSLoader
branch
from
9c76a06
to
3b28f03
Compare
|
@srenatus do you want me to rebase and force push my branch? |
|
@simongottschlag thanks, I think we're good here. I've let github update from main, and I'll squash&merge when green. Thanks for your contribution 🚀 |
| } | ||
|
|
||
| fileName := d.files[d.idx] | ||
| d.idx++ |
There was a problem hiding this comment.
This incrementing is the only bit of this method that needs to be synchronized. Thus, it is probably better to use atomic.AddUint32 since that will avoid the mutex alltogether. In order for this to work, the addition must go at the top of the method, before it is used.
| sync.Mutex | ||
| filesystem fs.FS | ||
| files []string | ||
| idx int |
There was a problem hiding this comment.
Nitpick: array indices cannot be negative, so this should be a uint.
|
@bittrance good points, feel free to do a follow-up 😉 |
To make it possible to create a DirectoryLoader based on the fs.FS interface.
This interface requires go version 1.16 or above.
Fixes: #3489
Signed-off-by: Simon Gottschlag simon.gottschlag@xenit.se