Scaffold S3 remote persistence interface #3
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
The new `RemotePersistence` trait provides an interface for loading and storing files on in remote location. The present implementation includes one implementation of this interface for AWS's S3 service. The `RemotePersistence` trait will optionally back the file-based persisters of results and manifests.
It appears that with ubuntu-latest runners, we can end up with queues much further away than we observed with rwx hosted runners. We should investigate what we can do to reduce this latency other than increasing batch sizes, but as a stopgap, bump the max overhead.
TAGraves
approved these changes
Apr 3, 2023
doxavore
reviewed
Apr 3, 2023
Comment on lines
+94
to
+95
| PersistenceKind::Manifest => "manifest", | ||
| PersistenceKind::Results => "results", |
There was a problem hiding this comment.
@ayazhafiz Do we want these to include a file extension, eg. .jsonl or .jsonl.gz?
There was a problem hiding this comment.
I don't have too much of a preference - what would you prefer?
There was a problem hiding this comment.
If it's easy enough to add in a follow-up, I like having the extension so we'll never need to guess the format.
There was a problem hiding this comment.
Okay, I'll be sure to add that in a next PR.
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.
The new
RemotePersistencetrait provides an interface for loading andstoring files on in remote location. The present implementation includes
one implementation of this interface for AWS's S3 service. The
RemotePersistencetrait will optionally back the file-based persistersof results and manifests.
Support for S3 is added as a feature, so that folks self-compiling ABQ do
not have to compile-in S3. By default, RWX will build ABQ binaries with
support for S3.
Note that the implementation adds a dependency on AWS's Rust SDK,
which is presently a "developer preview" and not stable. However,
anecdotal evidence suggests (1, 2)
the dependency is usable. We also do not have much other choice with
regards to AWS S3 clients.