Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(blob-store): Add wasi-blob-store capability #361

Merged
merged 25 commits into from
Mar 31, 2023
Merged

feat(blob-store): Add wasi-blob-store capability #361

merged 25 commits into from
Mar 31, 2023

Conversation

Mossaka
Copy link
Member

@Mossaka Mossaka commented Mar 14, 2023 •
edited
Loading

This PR is largely work-in-process. It's goal is to add wasi-blob-store proposal as a capability to slight.

I will make increment addition to this PR as seperate commits for easier review.

  • structure the blob-store WIT
  • implement AWS S3 on blob-store interface
  • write an example of using S3
  • write an integration test to fully test the APIs of blob store
  • do we want to implement available and close?

@Mossaka
added wits
cdbbd91 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka Mossaka marked this pull request as draft March 14, 2023 07:31
@Mossaka Mossaka added the ⭐ epic A grouping of features or tasks label Mar 14, 2023
@Mossaka Mossaka linked an issue Mar 14, 2023 that may be closed by this pull request
implement wasi-blob-store interface #275
Closed
@Mossaka Mossaka added this to the [KUBECON] March Milestone milestone Mar 14, 2023
@Mossaka
created file structure
8921231 
Signed-off-by: Jiaxiao Zhou <[email protected]>
danbugs
danbugs reviewed Mar 15, 2023
View reviewed changes
Copy link
Collaborator

@danbugs danbugs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this PR only for adding the interface or do you plan to add an implementor (i.e., possibly move azblob)?

@Mossaka
Copy link
Member Author

Mossaka commented Mar 15, 2023

Is this PR only for adding the interface or do you plan to add an implementor (i.e., possibly move azblob)?

I intended to work more on this PR to add at least one implementor. Possibly I will add aws s3

Mossaka added 6 commits March 20, 2023 23:08
@Mossaka
Merge branch 'main' into blob-store
481f3fe
@Mossaka
get the structure right
0623cfe 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
Implemented AWS S3 for Blob Store
2c69f8f 
Modify the the read stream APIs to change read-into to read due to the
current limitation of wit-bindgen which does not support generating
a mutable resource reference.
@Mossaka
nit: remove unused deps
cf5ca07
@Mossaka
Add blob-store to CLI and a demo to examples
270dd77 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
rustfmt
eb68392 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka Mossaka marked this pull request as ready for review March 24, 2023 22:39
@Mossaka
Copy link
Member Author

Mossaka commented Mar 24, 2023

This PR is mostly ready for review, although there are some remaining tasks to finish before this is mergable. I will document the tasks on the PR description section.

Mossaka added 5 commits March 24, 2023 16:04
@Mossaka
extract literal strings to const
b8d6d8e 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
clippy
07ac358 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
Add integration test for s3
2dcfa3b 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
resolve clippy issues
eb30d49 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
add comments && docs to blob-store
89c6ced 
Signed-off-by: Jiaxiao Zhou <[email protected]>
arschles
arschles reviewed Mar 27, 2023
View reviewed changes
Copy link
Contributor

@arschles arschles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nit-picks, but overall looks great

crates/blob-store/src/implementors/aws_s3.rs Outdated Show resolved Hide resolved
crates/blob-store/src/implementors/aws_s3.rs Outdated Show resolved Hide resolved
crates/blob-store/src/lib.rs Show resolved Hide resolved
tests/blob-store-test/src/main.rs Outdated Show resolved Hide resolved
tests/blob-store-test/src/main.rs Outdated Show resolved Hide resolved
tests/blob-store-test/src/main.rs Outdated Show resolved Hide resolved
devigned
devigned reviewed Mar 27, 2023
View reviewed changes
Copy link
Member

@devigned devigned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nits and questions. Looking pretty solid.

wit/blob-types.wit Show resolved Hide resolved
wit/blob-types.wit Show resolved Hide resolved
wit/blob-store.wit Show resolved Hide resolved
wit/blob-store.wit Show resolved Hide resolved
tests/blob-store-test/testfile.txt Outdated Show resolved Hide resolved
crates/blob-store/src/read_stream.rs Outdated Show resolved Hide resolved
crates/blob-store/src/implementors/aws_s3.rs Show resolved Hide resolved
crates/blob-store/src/implementors/aws_s3.rs Outdated Show resolved Hide resolved
Mossaka added 2 commits March 29, 2023 12:34
@Mossaka
feat(blob-store): Add Azure Blob Storage implementation to wasi-blob-…
cb10314 
…store capability (#372)

add azure blob storage implementation to wasi-blob-store capability

Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
fmt
a50703a 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
Copy link
Member Author

Mossaka commented Mar 29, 2023 •
edited
Loading

I've merged the azure-blob implementation to this PR and unforunately this one became much harder to review... I would really appreicate if you could take a second look @devigned, @danbugs, @arschles

devigned
devigned requested changes Mar 29, 2023
View reviewed changes
Copy link
Member

@devigned devigned left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

azblob implementation looks good. Nice work. I'm unsure as to what the expected behavior is of close and available. I have ideas about what behavior is expect of an implementor, but it would helpful to be clear about it.

crates/blob-store/src/read_stream.rs Show resolved Hide resolved
crates/blob-store/src/write_stream.rs Show resolved Hide resolved
Mossaka added 3 commits March 29, 2023 16:46
@Mossaka
Use testfiles from the fs as blobs
30711f8 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
Add comments to read-and-write streams
6fd05c9 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
fmt
d344a97 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
clippy
fc256d5 
Signed-off-by: Jiaxiao Zhou <[email protected]>
@Mossaka
Copy link
Member Author

Mossaka commented Mar 30, 2023 •
edited
Loading

Could you please give another pass? @devigned , @danbugs . I'd like to get this merged in today

@Mossaka
Copy link
Member Author

Mossaka commented Mar 31, 2023

I will merge this PR in and have a follow-up PR to fix the delete-objects and keep wits up-to-date.

@Mossaka Mossaka merged commit e0fbb96 into main Mar 31, 2023
@Mossaka Mossaka deleted the blob-store branch March 31, 2023 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danbugs danbugs danbugs left review comments

@arschles arschles arschles left review comments

@devigned devigned devigned requested changes

Assignees
No one assigned
Labels
⭐ epic A grouping of features or tasks
Projects
Wasm Projects
Status: Done
Milestone
Milestone
Development

Successfully merging this pull request may close these issues.

implement wasi-blob-store interface
4 participants
@Mossaka @arschles @devigned @danbugs