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

Fix bad bucketname in storage usage. #800

Merged
merged 2 commits into from
Feb 12, 2014

Conversation

ksauzz
Copy link
Contributor

@ksauzz ksauzz commented Feb 7, 2014

This fixes the bug that bucket names in storage usage are wrong.

Bug report: #797

@ksauzz ksauzz added this to the 1.4.5 milestone Feb 7, 2014
@ksauzz ksauzz added the Bug label Feb 7, 2014
?assertEqual(ok, erlcloud_s3:create_bucket(Bucket, UserConfig)),
%% Put 100-byte object
Block = crypto:rand_bytes(100),
_ = erlcloud_s3:put_object(Bucket, ?KEY, Block, UserConfig),
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's verify this function returns [{version_id,"null"}].

@kuenishi
Copy link
Contributor

kuenishi commented Feb 7, 2014

  • dialyzer / xref
  • eunit / pulse
  • r_t
  • review

@@ -61,7 +61,7 @@ sum_user(Riak, User) when is_list(User) ->
{string(), term()|string()}.
maybe_sum_bucket(Riak, User, Bucket) ->
case sum_bucket(Riak, Bucket) of
{struct, _} = BucketUsage -> BucketUsage;
{struct, _} = BucketUsage -> {Bucket, BucketUsage};
Copy link
Contributor

Choose a reason for hiding this comment

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

Any ideas why dialyzer didn't catch this before?

Copy link
Contributor

Choose a reason for hiding this comment

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

See #759, probably I didn't run dialyzer after fixing this function's spec. Maybe current spec notation is too loose, so more strict type/spec setting may improve safety.

@kuenishi
Copy link
Contributor

kuenishi commented Feb 7, 2014

Too late to wait for r_t finish, will resume tomorrow or next Monday.

confirm() ->
Config = [{riak, rtcs:riak_config()}, {stanchion, rtcs:stanchion_config()},
{cs, rtcs:cs_config([{fold_objects_for_list_keys, true}])}],
{UserConfig, {_RiakNodes, _CSNodes, _Stanchion}} = rtcs:setup(4, Config),
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend rtcs:setup(1, Config) for shorter startup/teardown time.

@kuenishi
Copy link
Contributor

kuenishi commented Feb 8, 2014

Code seems obvious and good, while dialyzer's behaviour still seems weird. This is a good change to refine type/spec notations around sum_user/2 .

@ksauzz ksauzz self-assigned this Feb 8, 2014
@ksauzz
Copy link
Contributor Author

ksauzz commented Feb 8, 2014

Thank you for the comments. I updated lines and also executed the kota's code formatter to storage_stats_test.erl.

@kuenishi
Copy link
Contributor

+1

ksauzz added a commit that referenced this pull request Feb 12, 2014
…ge-stats

Fix bad bucketname in storage usage.
@ksauzz ksauzz merged commit 81b518e into release/1.4 Feb 12, 2014
@ksauzz ksauzz deleted the bugfix/fix-bad-bucketname-in-storage-stats branch February 12, 2014 04:23
reiddraper added a commit that referenced this pull request Feb 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants