Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Fix list-objects returning nothing after restarting #220

Closed
wants to merge 2 commits into from

Conversation

camjackson
Copy link

Currently if fake-s3 is restarted, it always returns an empty response for the list-objects action. However, the objects are still there and can be fetched with get-object if you already know the key.

This PR is similar to this one, but including the extra fixes in comments from @Allactaga and @mark2997.

Copy link
Owner

@jubos jubos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Mainly some style nits before it can be merged. Can you also squash your diff into one logical change instead of 2 diffs?

@@ -278,5 +278,11 @@ def create_metadata(content,request)
end
return metadata
end
private
Copy link
Owner

Choose a reason for hiding this comment

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

Style nit: Indentation looks off here.

@@ -21,7 +21,8 @@ def initialize(root)
@bucket_hash = {}
Dir[File.join(root,"*")].each do |bucket|
bucket_name = File.basename(bucket)
bucket_obj = Bucket.new(bucket_name,Time.now,get_objects(bucket_name, bucket))
puts "root bucket: #{bucket_name}"
Copy link
Owner

Choose a reason for hiding this comment

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

Leave the puts out to avoid console spew.

get_object(bucket_name, File.basename(filepath), nil)
end
end
private
Copy link
Owner

Choose a reason for hiding this comment

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

Add a newline here.

end
end
private
def get_objects bucket_name, path, root_path=nil
Copy link
Owner

Choose a reason for hiding this comment

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

Use 2 spaces for indentation and () in the method declaration like the other methods in the class.

@camjackson
Copy link
Author

Thanks for your feedback @jubos! I actually made this PR from @saltzmanjoelh's fork (I didn't even know you could do a PR from a repo you don't own), which means I'm not able to push more code in order to amend this commit.

Instead I've opened a new PR, #221, which hopefully addresses your comments, and I'll close this one.

@camjackson camjackson closed this Oct 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants