-
Notifications
You must be signed in to change notification settings - Fork 217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(storage/fs): change fs.FSSource into fs.SnapshotSource #2325
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
Uffizzi Preview |
markphelps
approved these changes
Nov 1, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love this abstraction!
Codecov Report
@@ Coverage Diff @@
## main #2325 +/- ##
==========================================
+ Coverage 70.87% 70.92% +0.05%
==========================================
Files 78 78
Lines 7436 7449 +13
==========================================
+ Hits 5270 5283 +13
+ Misses 1866 1865 -1
- Partials 300 301 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
markphelps
added a commit
that referenced
this pull request
Nov 1, 2023
* 'main' of https://github.com/flipt-io/flipt: refactor(storage/fs): change fs.FSSource into fs.SnapshotSource (#2325)
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.
This refactors the
fs.Store
implementation to work over a new typefs.SnapshoutSource
instead offs.FSSource
.The difference between these abstractions is instead of returning
fs.FS
interface implementations, this source returns the full snapshots. The benefit here is the source can decide how to build the snapshot. Instead of having to conform tofs.FS
. The storagefs.SnapshotFromFS
can still be used to turn anyfs.FS
into a snapshot.All the existing sources have just been updated to call this utility function to adapt their existing
fs.FS
implementations.Future implementations don't have to use this abstraction and we can change existing ones if and when it makes sense.
The last thing it does is expose a new
SnapshotFromFiles
method.This method just takes a slice of
fs.File
instead of the fullfs.FS
.The
fs.File
interface is actually quite appropriate and more approachable than the fullfs.FS
abstraction.It will be necessary for both reading contents and for identifying file extension (to support both Yaml and JSON).
The OCI implementation will skip the
fs.FS
abstraction and use just thefs.File
implementation instead.I can update all the open FS issues to reference these new abstractions instead.