Skip to content

Commit

Permalink
Update comments and docstr for core move and copy
Browse files Browse the repository at this point in the history
  • Loading branch information
cslzchen committed Jun 19, 2018
1 parent d3814ec commit a2d4f2c
Showing 1 changed file with 23 additions and 31 deletions.
54 changes: 23 additions & 31 deletions waterbutler/core/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,8 @@ async def make_request(self, method: str, url: str, *args, **kwargs) -> aiohttp.
def request(self, *args, **kwargs):
return RequestHandlerContext(self.make_request(*args, **kwargs))

async def move(self,
dest_provider: 'BaseProvider',
src_path: wb_path.WaterButlerPath,
dest_path: wb_path.WaterButlerPath,
rename: str=None,
conflict: str='replace',
async def move(self, dest_provider: 'BaseProvider', src_path: wb_path.WaterButlerPath,
dest_path: wb_path.WaterButlerPath, rename: str=None, conflict: str='replace',
handle_naming: bool=True) -> typing.Tuple[wb_metadata.BaseMetadata, bool]:
"""Moves a file or folder from the current provider to the specified one
Performs a copy and then a delete.
Expand All @@ -224,8 +220,9 @@ async def move(self,
'got_rename': rename is not None,
})

# This does not mean the `dest_path` is a folder, at this point it could just be the parent
# of the actual dest_path, or have a blank name
# This does not mean that the `dest_path` is a folder, at this point it could just be the
# parent of the actual destination file/folder, or have a blank name. Use `not .is_file`
# in the if statement for less confusion.
if not dest_path.is_file:
temp_path = await self.revalidate_path(
dest_path,
Expand All @@ -236,7 +233,6 @@ async def move(self,
raise exceptions.OverwriteSelfError(src_path)

if handle_naming:

dest_path = await dest_provider.handle_naming(
src_path,
dest_path,
Expand All @@ -248,8 +244,8 @@ async def move(self,

# files and folders shouldn't overwrite themselves
if (
self.shares_storage_root(dest_provider) and
src_path.materialized_path == dest_path.materialized_path
self.shares_storage_root(dest_provider) and
src_path.materialized_path == dest_path.materialized_path
):
raise exceptions.OverwriteSelfError(src_path)

Expand All @@ -267,13 +263,10 @@ async def move(self,

return meta_data, created

async def copy(self,
dest_provider: 'BaseProvider',
src_path: wb_path.WaterButlerPath,
dest_path: wb_path.WaterButlerPath,
rename: str=None, conflict: str='replace',
handle_naming: bool=True) \
-> typing.Tuple[wb_metadata.BaseMetadata, bool]:
async def copy(self, dest_provider: 'BaseProvider', src_path: wb_path.WaterButlerPath,
dest_path: wb_path.WaterButlerPath, rename: str=None, conflict: str='replace',
handle_naming: bool=True) -> typing.Tuple[wb_metadata.BaseMetadata, bool]:

args = (dest_provider, src_path, dest_path)
kwargs = {'rename': rename, 'conflict': conflict, 'handle_naming': handle_naming}

Expand All @@ -283,8 +276,9 @@ async def copy(self,
'got_rename': rename is not None,
})

# This does not mean the `dest_path` is a folder, at this point it could just be the parent
# of the actual dest_path, or have a blank name
# This does not mean that the `dest_path` is a folder, at this point it could just be the
# parent of the actual destination file/folder, or have a blank name. Use `not .is_file`
# in the if statement for less confusion.
if not dest_path.is_file:
temp_path = await self.revalidate_path(
dest_path,
Expand All @@ -295,7 +289,6 @@ async def copy(self,
raise exceptions.OverwriteSelfError(src_path)

if handle_naming:

dest_path = await dest_provider.handle_naming(
src_path,
dest_path,
Expand Down Expand Up @@ -399,7 +392,8 @@ async def handle_naming(self,
dest_path: wb_path.WaterButlerPath,
rename: str=None,
conflict: str='replace') -> wb_path.WaterButlerPath:
"""Given a :class:`.WaterButlerPath` and the desired name, handle any potential naming issues.
"""Given a :class:`.WaterButlerPath` and the desired name, handle any potential naming
issues.
i.e.:
Expand All @@ -413,32 +407,30 @@ async def handle_naming(self,
:param src_path: ( :class:`.WaterButlerPath` ) The object that is being copied
:param dest_path: ( :class:`.WaterButlerPath` ) The path that is being copied to or into
:param dest_path: ( :class:`.WaterButlerPath` ) The path that is being copied TO or INTO
:param rename: ( :class:`str` ) The desired name of the resulting path, may be incremented
:param conflict: ( :class:`str` ) The conflict resolution strategy, ``replace`` or ``keep``
:rtype: :class:`.WaterButlerPath`
"""

# This function can either take a file path, or a parent path to make a file path out of.

# Can't copy a directory to a file
if src_path.is_dir and dest_path.is_file:
# Cant copy a directory to a file
raise ValueError('Destination must be a directory if the source is')

# This is confusing. `dest_path` at this point can refer to the parent or root of
# the file we want to move/copy. So even if moving/copying a file, this code will run
if not dest_path.is_file:
# Directories always are going to be copied into
# cp /folder1/ /folder2/ -> /folder1/folder2/
# At this point, `dest_path` must be the parent or root folder into which we want to
# move/copy the source. The file/folder is always going to be copied into the parent.
# For example:
# cp /file.txt /folder/ -> /folder/file.txt
# cp /folder1/ /folder2/ -> /folder1/folder2/
dest_path = await self.revalidate_path(
dest_path,
rename or src_path.name,
folder=src_path.is_dir
)

dest_path, _ = await self.handle_name_conflict(dest_path, conflict=conflict)

return dest_path

def will_self_overwrite(self,
Expand Down

0 comments on commit a2d4f2c

Please sign in to comment.