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

rgw: dump s3_code as the Code response element in RGWDeleteMultiObj_ObjStore_S3 #12470

Merged
merged 1 commit into from
Jun 3, 2019
Merged

rgw: dump s3_code as the Code response element in RGWDeleteMultiObj_ObjStore_S3 #12470

merged 1 commit into from
Jun 3, 2019

Conversation

rzarzynski
Copy link
Contributor

@rzarzynski rzarzynski commented Dec 13, 2016 •
edited
Loading

@rzarzynski
rgw: dump s3_code as the Code response element in RGWDeleteMultiObj_O…
8406083 
…bjStore_S3.

Fixes: http://tracker.ceph.com/issues/18241
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
yehudasa
yehudasa reviewed Dec 13, 2016
View reviewed changes
Copy link
Member

@yehudasa yehudasa left a comment

Choose a reason for hiding this comment

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

fix is fine, but need to add a test case in s3-tests

@liuchang0812
Copy link
Contributor

@rzarzynski @yehudasa I have submitted another PR about this #12485

I would like to fix this bug, and add some test cases. How about?

@rzarzynski
Copy link
Contributor Author

@liuchang0812: I'm completely fine if you want to take care of this bug. :-) Just please be aware there is an ongoing rework in the area of error handling.

@liuchang0812
Copy link
Contributor

@rzarzynski thanks. I will take care of the ongoing rework. I am very pleasure to take part in this work.

@liuchang0812
Copy link
Contributor

@rzarzynski I closed #12485 . This PR could be ongoing. very thanks

code-with-amitk
code-with-amitk approved these changes Aug 22, 2017
View reviewed changes
src/rgw/rgw_rest_s3.cc
@@ -2882,7 +2882,7 @@ void RGWDeleteMultiObj_ObjStore_S3::send_partial_response(rgw_obj_key& key,

s->formatter->dump_string("Key", key.name);
s->formatter->dump_string("VersionId", key.instance);
s->formatter->dump_int("Code", r.http_ret);
s->formatter->dump_string("Code", r.s3_code);
Copy link

Choose a reason for hiding this comment

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

@rzarzynski (NIT) Please consider to keep PR title < 50 characters for future PRs.
https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst#title-of-pull-requests-and-title-of-commits

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Amit,

Sometimes exceeding 50 characters in a PR title (or bz subject) is probably justified. That's something that the RGW reviewers would probably flag, if it were a problem, in the case of an RGW PR.

cheers,

Matt

Matt

Copy link

Choose a reason for hiding this comment

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

@mattbenjamin Thanks.
Was just an friendly reminder if developer skipped at time of coding.

@stale
Copy link

stale bot commented Oct 18, 2018

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Oct 18, 2018
@stale stale bot removed the stale label Oct 31, 2018
@mattbenjamin mattbenjamin self-assigned this Oct 31, 2018
mattbenjamin
mattbenjamin approved these changes Oct 31, 2018
View reviewed changes
Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

trivial and still seems relevant

@rzarzynski
Copy link
Contributor Author

jenkins retest this please (no log)

@stale
Copy link

stale bot commented Jan 2, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you are a maintainer or core committer, please follow-up on this issue to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@stale stale bot added the stale label Jan 2, 2019
@tchaikov
Copy link
Contributor

tchaikov commented Apr 5, 2019

@yuriw could you include this change in your next batch?

@stale stale bot removed the stale label Apr 5, 2019
@cbodley cbodley merged commit eb51579 into ceph:master Jun 3, 2019
@cbodley
Copy link
Contributor

cbodley commented Jun 3, 2019

passed qa in http://pulpito.ceph.com/cbodley-2019-05-30_18:03:29-rgw-wip-cbodley-testing-distro-basic-smithi/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yehudasa yehudasa yehudasa left review comments

@code-with-amitk code-with-amitk code-with-amitk approved these changes

@mattbenjamin mattbenjamin mattbenjamin approved these changes

Assignees

@mattbenjamin mattbenjamin

Labels
bug-fix needs-qa rgw wip-cbodley-testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rzarzynski @liuchang0812 @tchaikov @cbodley @mattbenjamin @yehudasa @code-with-amitk