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

Bug Fix #7328 | Remove Duplications before Sending Them to Delete Multiple Objects #7329

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

shirady
Copy link
Contributor

@shirady shirady commented Jun 7, 2023

Explain the changes

  1. Remove duplications before sending them to delete multiple objects.
  2. Add parsing XML errors in case required parts the XML were not sent by the client.

Issues: Fixed #7328

  1. Delete-objects reply of the same key objects should be the same as AWS.

  2. If the client didn't send the XML with the Delete part we would have an error that we cannot read properties of undefined (tried to read 'Quiet'). Note: This was not part of the original issue but was reviled in the discussion of the PR.

Testing Instructions:

  1. Are inside the issue mentioned above Delete Objects Reply of Same Key Objects Using Noobaa Endpoint Is Different than AWS #7328 .
  • Doc added/updated
  • Tests added

@shirady shirady self-assigned this Jun 7, 2023
@nimrod-becker nimrod-becker requested review from jackyalbo, a team, liranmauda, romayalon and achouhan09 and removed request for a team, liranmauda and romayalon June 7, 2023 08:04
@shirady shirady marked this pull request as draft June 7, 2023 08:10
@shirady shirady force-pushed the fix-delete-objects branch 3 times, most recently from 9a2279b to f02d5f6 Compare June 8, 2023 12:52
@pull-request-size pull-request-size bot added size/XS and removed size/S labels Jun 8, 2023
@shirady shirady marked this pull request as ready for review June 8, 2023 13:45
@shirady shirady requested a review from achouhan09 June 8, 2023 13:45
Copy link
Contributor

@jackyalbo jackyalbo left a comment

Choose a reason for hiding this comment

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

Wouldn't it be better to remove duplicates from the request? before sending to the core? and then avoid it for the results?

@shirady
Copy link
Contributor Author

shirady commented Jun 11, 2023

Wouldn't it be better to remove duplicates from the request? before sending to the core? and then avoid it for the results?

Thank you, I changed it.
As far as I know, it would cause failed tests of src/test/unit_tests/test_bucketspace_versioning.js since there are tests that count the keys in the result.

@shirady shirady marked this pull request as ready for review June 12, 2023 13:35
@shirady shirady requested a review from romayalon June 12, 2023 13:38
@romayalon
Copy link
Contributor

@shirady @jackyalbo

Wouldn't it be better to remove duplicates from the request? before sending to the core? and then avoid it for the results?

it's not just better but it's a must... the problem is not within the result array but within the request as Jacky mentioned,
I checked deleting multiple objects scenario against Amazon while bucket versioning is enabled, S3 will create only one delete marker, which means that AWS will not delete the object multiple times, so we shouldn't as well

@shirady
Copy link
Contributor Author

shirady commented Jun 12, 2023

@shirady @jackyalbo

Wouldn't it be better to remove duplicates from the request? before sending to the core? and then avoid it for the results?

it's not just better but it's a must... the problem is not within the result array but within the request as Jacky mentioned, I checked deleting multiple objects scenario against Amazon while bucket versioning is enabled, S3 will create only one delete marker, which means that AWS will not delete the object multiple times, so we shouldn't as well

@romayalon Thank you.
The current changes you see in this PR changes are after I took into account what @jackyalbo suggested.

  • I will update the title in case it's confusing.

@shirady shirady changed the title Bug Fix #7328 | Change the Result Array of Delete Objects Bug Fix #7328 | Remove Duplications before Sending It to Delete Multiple Objects Jun 12, 2023
@shirady shirady force-pushed the fix-delete-objects branch 5 times, most recently from b548ab4 to 1232bc3 Compare June 29, 2023 07:11
@shirady shirady requested a review from guymguym June 29, 2023 07:11
and add parsing XML errors

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
@shirady shirady changed the title Bug Fix #7328 | Remove Duplications before Sending It to Delete Multiple Objects Bug Fix #7328 | Remove Duplications before Sending Them to Delete Multiple Objects Jun 29, 2023
@shirady shirady merged commit f334ffe into noobaa:master Jun 29, 2023
6 checks passed
@shirady shirady deleted the fix-delete-objects branch July 2, 2023 08:36
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.

Delete Objects Reply of Same Key Objects Using Noobaa Endpoint Is Different than AWS
6 participants