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

Option to move restored data to origins. #11

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

Conversation

karitra
Copy link
Contributor

@karitra karitra commented Jul 1, 2019

TODO:

  • fix test.

@karitra karitra added the WIP Work in progress label Jul 1, 2019
@karitra karitra mentioned this pull request Jul 1, 2019
1 task
@karitra karitra force-pushed the pr/move_to_source branch 7 times, most recently from bb64c44 to 857f75b Compare July 2, 2019 18:39
@karitra karitra removed the WIP Work in progress label Jul 2, 2019
@karitra karitra requested a review from shaitan July 2, 2019 18:39
@karitra karitra force-pushed the pr/move_to_source branch from 857f75b to 1ca3cdb Compare July 2, 2019 18:58
eblob_kit.py Show resolved Hide resolved
eblob_kit.py Show resolved Hide resolved
eblob_kit.py Outdated Show resolved Hide resolved
eblob_kit.py Outdated
@staticmethod
def move_back(from_index, to_index):
"""Move underlying file(s) between specified pathes."""
if not to_index.endswith(SORTED_INDEX_SUFFIX):
Copy link
Collaborator

Choose a reason for hiding this comment

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

if from_index.endswith(SORTED_INDEX_SUFFIX) == to_index.endswith(SORTED_INDEX_SUFFIX):
    shutil.move(src=from_index, dst=to_index)
else:
    temporary_name = to_index + TO_REMOVE_SUFFIX
    shutil.move(src=to_index, dst=temporary_name)
    shutil.move(src=from_index, dst=to_index)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

eblob_kit.py Outdated Show resolved Hide resolved
eblob_kit.py Outdated Show resolved Hide resolved
eblob_kit.py Outdated
self._file.close()

@staticmethod
def move_back(from_index, to_index):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why isn't it just a method of IndexFile:

def move(self, path):
    shutil.move(self.path, path)

Copy link
Contributor Author

@karitra karitra Jul 3, 2019

Choose a reason for hiding this comment

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

It was implemented likewise in first version of this PR, but it seems a violation of class api, as 'user' can freely call move for entity in wrong state, e.g. before any file was closed or written. It could be combined close_and_move method here, but it looks more rigid then current version.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be made via close, move, re-open

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also it could be made without close and re-open within one filesystem via os.rename().

eblob_kit.py Show resolved Hide resolved
eblob_kit.py Outdated Show resolved Hide resolved
@karitra karitra force-pushed the pr/move_to_source branch 3 times, most recently from 4f03d3b to ab3ca41 Compare July 3, 2019 19:45
@karitra
Copy link
Contributor Author

karitra commented Jul 3, 2019

Updated, based on review. @shaitan, PTAL.

@karitra karitra force-pushed the pr/move_to_source branch from 132a529 to 0d9e2b1 Compare July 5, 2019 10:22
@karitra karitra force-pushed the pr/move_to_source branch from 0d9e2b1 to 81e86a9 Compare August 9, 2019 15:54
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.

2 participants