From d795cdf1bae8fd4b8a2fe795dd6ccfd0bd89818d Mon Sep 17 00:00:00 2001
From: Ruslan Kuprieiev <ruslan@iterative.ai>
Date: Fri, 21 Jun 2019 12:03:50 +0300
Subject: [PATCH] dvc: address review from #2160

Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>
---
 dvc/external_repo.py | 24 +++++++++++-------------
 dvc/repo/get_url.py  |  4 ++--
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/dvc/external_repo.py b/dvc/external_repo.py
index ab8b0e493..7b9288751 100644
--- a/dvc/external_repo.py
+++ b/dvc/external_repo.py
@@ -1,8 +1,9 @@
+from __future__ import unicode_literals
+
 import os
-import uuid
-import errno
 import shutil
 import logging
+import shortuuid
 
 from funcy import cached_property
 from schema import Optional
@@ -10,6 +11,7 @@ from schema import Optional
 from dvc.config import Config
 from dvc.cache import CacheConfig
 from dvc.exceptions import DvcException
+from dvc.utils.compat import makedirs
 
 
 logger = logging.getLogger(__name__)
@@ -98,9 +100,6 @@ class ExternalRepo(object):
             cache_config.set_dir(cache_dir, level=Config.LEVEL_LOCAL)
             repo.scm.git.close()
 
-        if self.installed:
-            self.uninstall()
-
     def install(self, cache_dir=None, force=False):
         if self.installed and not force:
             logger.info(
@@ -109,18 +108,14 @@ class ExternalRepo(object):
             )
             return
 
-        try:
-            os.makedirs(self.repos_dir)
-        except OSError as exc:
-            if exc.errno != errno.EEXIST:
-                raise
+        makedirs(self.repos_dir, exist_ok=True)
 
         # installing package to a temporary directory until we are sure that
         # it has been installed correctly.
         #
-        # Note that tempfile.TemporaryDirectory is using symlinks to tmpfs, so
-        # we won't be able to use move properly.
-        tmp_dir = os.path.join(self.repos_dir, "." + str(uuid.uuid4()))
+        # Note that we can't use tempfile.TemporaryDirectory is using symlinks
+        # to tmpfs, so we won't be able to use move properly.
+        tmp_dir = os.path.join(self.repos_dir, "." + str(shortuuid.uuid()))
         try:
             self._install_to(tmp_dir, cache_dir)
         except ExternalRepoError:
@@ -128,6 +123,9 @@ class ExternalRepo(object):
                 shutil.rmtree(tmp_dir)
             raise
 
+        if self.installed:
+            self.uninstall()
+
         shutil.move(tmp_dir, self.path)
 
     def uninstall(self):
diff --git a/dvc/repo/get_url.py b/dvc/repo/get_url.py
index 7ba65c4c7..ca9663746 100644
--- a/dvc/repo/get_url.py
+++ b/dvc/repo/get_url.py
@@ -11,9 +11,9 @@ def get_url(url, out=None):
     out = out or os.path.basename(urlparse(url).path)
 
     if os.path.exists(url):
-        src = os.path.abspath(url)
+        url = os.path.abspath(url)
         out = os.path.abspath(out)
 
-    dep, = dependency.loads_from(None, [src])
+    dep, = dependency.loads_from(None, [url])
     out, = output.loads_from(None, [out], use_cache=False)
     dep.download(out)