Skip to content

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 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
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().

@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