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.

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

Update README #11

Merged
merged 3 commits into from
Jun 20, 2024
Merged

Update README #11

merged 3 commits into from
Jun 20, 2024

Conversation

pablomatiasgomez
Copy link
Collaborator

Added a better description of why this module might be useful and in which cases.

@pablomatiasgomez pablomatiasgomez mentioned this pull request Jun 19, 2024
README.md Outdated
@@ -62,14 +73,14 @@ func main() {
"retention-days": "14",
})
if err != nil {
panic("unable to create temp file: " + err.Error())
panic("failed to create temp file: " + err.Error())
}

// Append all the objects to the file:
for id, obj := range objects {
err = file.Append(id, obj)
if err != nil {

Choose a reason for hiding this comment

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

I would do:

if err  := file.Append(id, obj); err != nil {
    panic(.....)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed!

Copy link

@fnewberg fnewberg left a comment

Choose a reason for hiding this comment

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

one minor suggestion

is placed in the file, and then upload one single file to s3 with many objects in it, reducing drastically the number
of PUT operations needed to store a large number of objets.

Then, when you need to retrieve an object, you can use the index information to fetch that object and the GET call to s3

Choose a reason for hiding this comment

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

should we add something about storing on the index data being beyond the scope of what this repo does, e.g.

The best solution for how to store and query your index data will depend on your application data patterns and is beyond the scope of what this package aspires to do. 

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, will add!

@pablomatiasgomez pablomatiasgomez merged commit a773ccb into master Jun 20, 2024
1 check passed
@pablomatiasgomez pablomatiasgomez deleted the pablomatiasgomez/update-readme branch June 20, 2024 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants