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

Avoid duplicate with testcase #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vikas-nautiyal
Copy link

This is the addition to the PR - #10
Added test case over the existing commit.

@marxjohnson
Copy link

marxjohnson commented Feb 14, 2022

I just ran a test of this an it worked as expected. Here's the steps I followed:

  • Upload 9 files to my bucket with versioning enabled at 11:45
  • Wait one minute
  • Delete one file
  • Upload a 10th file
  • Run s3-pit-restore -b bucketname -B bucketname -t "11:46:00 14-02-2022 +0" --avoid-duplicates
  • Result: The deleted file is restored, the extra file is set to a delete marker. All other files are left as a single version. Only the changed files are output.

We are working with buckets holding 4 million objects and 6TB of data, so this feature is an absolute must for us. Thank you for working on it!

@angeloc
Copy link
Owner

angeloc commented Feb 14, 2022

@marxjohnson Could you also do a run of the software testing suite?

@marxjohnson
Copy link

I ran:
py ./s3-pit-restore -b mj5982-pit -B mj5982-pit -P restore-path --test
And I got:

======================================================================
FAIL: test_avoid_duplicates (__main__.TestS3PitRestore)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\mj5982\src\s3-pit-restore\s3-pit-restore", line 257, in test_avoid_duplicates
    self.assertTrue(self.compare_versions(version_before, version_after))
AssertionError: False is not true

----------------------------------------------------------------------
Ran 3 tests in 273.167s

FAILED (failures=1)

So something might not be quite right there. @vikas-nautiyal Any thoughts on why it's failing?

Fix getting file versions when dest_prefix is provided.
@angeloc
Copy link
Owner

angeloc commented Nov 17, 2022

@marxjohnson could you do a test run with the testing suite?

@vikas-nautiyal
Copy link
Author

vikas-nautiyal commented Nov 17, 2022

Hi @marxjohnson, added a fix to the code. It was failing because the -P option would restore the files in a different directory i.e dest_prefix. added fix to verify if the restore directory is different than the root of the bucket and act accordingly.

@angeloc
Copy link
Owner

angeloc commented Apr 21, 2023

@vikas-nautiyal could you rebase the MR, I'll have a run and merge if it's ok. Thanks!

@angeloc
Copy link
Owner

angeloc commented Jan 17, 2024

@vikas-nautiyal It would be nice if you could rebase this PR resolving conflicts so I can merge it. Thanks!

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.

6 participants