Repository: conan-io/conan

Issue:
KeyError on second upload of a package without remote in registry
When a package is uploaded to a remote, it gets associated to it in the registry. If the remote is removed from the registry (``conan remote remove``) and the package is uploaded to a different remote, Conan ends with an error and the name of the remote.

The error turns to be a KeyError on a registry search of the remote url and verify_ssl in here: https://github.com/conan-io/conan/blob/a52dcdc87dd8146b661ed650da616b9fcd2371e3/conans/client/cache/remote_registry.py#L122

Here is a test reproducing the issue:

```
    def upload_key_error_test(self):
        files = cpp_hello_conan_files("Hello0", "1.2.1", build=False)
        server1 = TestServer([("*/*@*/*", "*")], [("*/*@*/*", "*")],
                                     users={"lasote": "mypass"})
        server2 = TestServer([("*/*@*/*", "*")], [("*/*@*/*", "*")],
                                     users={"lasote": "mypass"})
        servers = OrderedDict()
        servers["server1"] = server1
        servers["server2"] = server2
        client = TestClient(servers=servers)
        client.save(files)
        client.run("config set general.revisions_enabled=True")
        client.run("create . user/testing")
        client.run("user lasote -p mypass")
        client.run("user lasote -p mypass -r server2")
        client.run("upload Hello0/1.2.1@user/testing --all -r server1")
        client.run("remove * --force")
        client.run("install Hello0/1.2.1@user/testing -r server1")
        client.run("remote remove server1")
        client.run("upload Hello0/1.2.1@user/testing --all -r server2")
        self.assertNotIn("ERROR: 'server1'")
```

Discussion:

Patch:
diff --git a/conans/client/cache/remote_registry.py b/conans/client/cache/remote_registry.py
index 37529e34b..61d2a42de 100644
--- a/conans/client/cache/remote_registry.py
+++ b/conans/client/cache/remote_registry.py
@@ -221,6 +221,7 @@ class _RemotesRegistry(_Registry):
                 raise ConanException("Remote '%s' not found in remotes" % remote_name)
             del remotes[remote_name]
             refs = {k: v for k, v in refs.items() if v != remote_name}
+            prefs = {k: v for k, v in prefs.items() if v != remote_name}
             self._save(remotes, refs, prefs)
 
     def clean(self):
diff --git a/conans/client/cmd/user.py b/conans/client/cmd/user.py
index 1738d986f..08daf29a2 100644
--- a/conans/client/cmd/user.py
+++ b/conans/client/cmd/user.py
@@ -6,7 +6,7 @@ def users_list(localdb_file, remotes):
     if not remotes:
         raise ConanException("No remotes defined")
 
-    localdb = LocalDB(localdb_file)
+    localdb = LocalDB.create(localdb_file)
     remotes_info = []
     for remote in remotes:
         user_info = {}
@@ -19,12 +19,11 @@ def users_list(localdb_file, remotes):
 
 
 def users_clean(localdb_file):
-    localdb = LocalDB(localdb_file)
-    localdb.init(clean=True)
+    LocalDB.create(localdb_file, clean=True)
 
 
 def user_set(localdb_file, user, remote_name=None):
-    localdb = LocalDB(localdb_file)
+    localdb = LocalDB.create(localdb_file)
 
     if user.lower() == "none":
         user = None
diff --git a/conans/client/conan_api.py b/conans/client/conan_api.py
index bd13f228f..9b4c9b244 100644
--- a/conans/client/conan_api.py
+++ b/conans/client/conan_api.py
@@ -158,7 +158,7 @@ class ConanAPIV1(object):
                                         revisions_enabled=cache.config.revisions_enabled,
                                         put_headers=put_headers)
         # To store user and token
-        localdb = LocalDB(cache.localdb)
+        localdb = LocalDB.create(cache.localdb)
         # Wraps RestApiClient to add authentication support (same interface)
         auth_manager = ConanApiAuthManager(rest_api_client, user_io, localdb)
         # Handle remote connections
diff --git a/conans/client/store/localdb.py b/conans/client/store/localdb.py
index 12895c7d8..4fec6f67f 100644
--- a/conans/client/store/localdb.py
+++ b/conans/client/store/localdb.py
@@ -3,6 +3,7 @@ import sqlite3
 from sqlite3 import OperationalError
 
 from conans.errors import ConanException
+from contextlib import contextmanager
 
 REMOTES_USER_TABLE = "users_remotes"
 
@@ -10,32 +11,28 @@ REMOTES_USER_TABLE = "users_remotes"
 class LocalDB(object):
 
     def __init__(self, dbfile):
+        self.dbfile = dbfile
+
+    @staticmethod
+    def create(dbfile, clean=False):
+        # Create the database file if it doesn't exist
         if not os.path.exists(dbfile):
             par = os.path.dirname(dbfile)
             if not os.path.exists(par):
                 os.makedirs(par)
             db = open(dbfile, 'w+')
             db.close()
-        self.dbfile = dbfile
-        try:
-            self.connection = sqlite3.connect(self.dbfile,
-                                              detect_types=sqlite3.PARSE_DECLTYPES)
-            self.connection.text_factory = str
-        except Exception as e:
-            raise ConanException('Could not connect to local cache', e)
-        self.init()
 
-    def init(self, clean=False):
-        cursor = None
+        connection = sqlite3.connect(dbfile, detect_types=sqlite3.PARSE_DECLTYPES)
+
         try:
-            cursor = self.connection.cursor()
+            cursor = connection.cursor()
             if clean:
                 cursor.execute("drop table if exists %s" % REMOTES_USER_TABLE)
                 try:
                     # https://github.com/ghaering/pysqlite/issues/109
-                    self.connection.isolation_level = None
+                    connection.isolation_level = None
                     cursor.execute('VACUUM')  # Make sure the DB is cleaned, drop doesn't do that
-                    self.connection.isolation_level = ''  # default value of isolation_level
                 except OperationalError:
                     pass
 
@@ -45,36 +42,46 @@ class LocalDB(object):
             message = "Could not initialize local sqlite database"
             raise ConanException(message, e)
         finally:
-            if cursor:
-                cursor.close()
+            connection.close()
 
-    def get_login(self, remote_url):
-        '''Returns login credentials.
-        This method is also in charge of expiring them.
-        '''
+        return LocalDB(dbfile)
+
+    @contextmanager
+    def _connect(self):
+        connection = sqlite3.connect(self.dbfile, detect_types=sqlite3.PARSE_DECLTYPES)
+        connection.text_factory = str
         try:
-            statement = self.connection.cursor()
-            statement.execute('select user, token from %s where remote_url="%s"'
-                              % (REMOTES_USER_TABLE, remote_url))
-            rs = statement.fetchone()
-            if not rs:
-                return None, None
-            name = rs[0]
-            token = rs[1]
-            return name, token
-        except Exception:
-            raise ConanException("Couldn't read login\n Try removing '%s' file" % self.dbfile)
+            yield connection
+        finally:
+            connection.close()
+
+    def get_login(self, remote_url):
+        """ Returns login credentials. This method is also in charge of expiring them. """
+        with self._connect() as connection:
+            try:
+                statement = connection.cursor()
+                statement.execute('select user, token from %s where remote_url="%s"'
+                                  % (REMOTES_USER_TABLE, remote_url))
+                rs = statement.fetchone()
+                if not rs:
+                    return None, None
+                name = rs[0]
+                token = rs[1]
+                return name, token
+            except Exception:
+                raise ConanException("Couldn't read login\n Try removing '%s' file" % self.dbfile)
 
     def get_username(self, remote_url):
         return self.get_login(remote_url)[0]
 
     def set_login(self, login, remote_url):
-        """Login is a tuple of (user, token)"""
-        try:
-            statement = self.connection.cursor()
-            statement.execute("INSERT OR REPLACE INTO %s (remote_url, user, token) "
-                              "VALUES (?, ?, ?)" % REMOTES_USER_TABLE,
-                              (remote_url, login[0], login[1]))
-            self.connection.commit()
-        except Exception as e:
-            raise ConanException("Could not store credentials %s" % str(e))
+        """ Login is a tuple of (user, token) """
+        with self._connect() as connection:
+            try:
+                statement = connection.cursor()
+                statement.execute("INSERT OR REPLACE INTO %s (remote_url, user, token) "
+                                  "VALUES (?, ?, ?)" % REMOTES_USER_TABLE,
+                                  (remote_url, login[0], login[1]))
+                connection.commit()
+            except Exception as e:
+                raise ConanException("Could not store credentials %s" % str(e))
diff --git a/conans/util/dates.py b/conans/util/dates.py
index d85af7088..c5935eb3a 100644
--- a/conans/util/dates.py
+++ b/conans/util/dates.py
@@ -7,7 +7,24 @@ def from_timestamp_to_iso8601(timestamp):
 
 
 def from_iso8601_to_datetime(iso_str):
-    return datetime.datetime.strptime(iso_str, '%Y-%m-%dT%H:%M:%SZ')
+
+    def transform_to_z_notation(the_str):
+        for p in ("+00:00", "+0000", "+00"):
+            if the_str.endswith(p):
+                the_str = the_str[0:-len(p)]
+                return "{}Z".format(the_str)
+        return the_str
+
+    base_pattern = "%Y-%m-%dT%H:%M:%S"
+    if "." in iso_str:
+        pattern = "{}.%fZ".format(base_pattern)
+    else:
+        pattern = '{}Z'.format(base_pattern)
+
+    iso_str = transform_to_z_notation(iso_str)
+    if not iso_str.endswith("Z"):
+        iso_str += "Z"
+    return datetime.datetime.strptime(iso_str, pattern)
 
 
 def iso8601_to_str(iso_str):
diff --git a/conans/util/env_reader.py b/conans/util/env_reader.py
index 05968a930..cbf46174e 100644
--- a/conans/util/env_reader.py
+++ b/conans/util/env_reader.py
@@ -25,5 +25,7 @@ def get_env(env_key, default=None, environment=None):
         elif isinstance(default, float):
             return float(env_var)
         elif isinstance(default, list):
-            return [var.strip() for var in env_var.split(",")]
+            if env_var.strip():
+                return [var.strip() for var in env_var.split(",")]
+            return []
     return env_var


Test patch:
diff --git a/conans/test/functional/command/upload_test.py b/conans/test/functional/command/upload_test.py
index f5a6c1304..d578998fa 100644
--- a/conans/test/functional/command/upload_test.py
+++ b/conans/test/functional/command/upload_test.py
@@ -1,6 +1,7 @@
 import itertools
 import os
 import unittest
+from collections import OrderedDict
 
 from mock import mock
 
@@ -540,3 +541,25 @@ class Pkg(ConanFile):
         self.assertIn("Uploading conanmanifest.txt", client.out)
         self.assertIn("Uploading conanfile.py", client.out)
         self.assertIn("Uploading conan_export.tgz", client.out)
+
+    def upload_key_error_test(self):
+        files = cpp_hello_conan_files("Hello0", "1.2.1", build=False)
+        server1 = TestServer([("*/*@*/*", "*")], [("*/*@*/*", "*")],
+                                     users={"lasote": "mypass"})
+        server2 = TestServer([("*/*@*/*", "*")], [("*/*@*/*", "*")],
+                                     users={"lasote": "mypass"})
+        servers = OrderedDict()
+        servers["server1"] = server1
+        servers["server2"] = server2
+        client = TestClient(servers=servers)
+        client.save(files)
+        client.run("config set general.revisions_enabled=True")
+        client.run("create . user/testing")
+        client.run("user lasote -p mypass")
+        client.run("user lasote -p mypass -r server2")
+        client.run("upload Hello0/1.2.1@user/testing --all -r server1")
+        client.run("remove * --force")
+        client.run("install Hello0/1.2.1@user/testing -r server1")
+        client.run("remote remove server1")
+        client.run("upload Hello0/1.2.1@user/testing --all -r server2")
+        self.assertNotIn("ERROR: 'server1'", client.out)
diff --git a/conans/test/integration/revisions_test.py b/conans/test/integration/revisions_test.py
index 4202269eb..93ee17a1c 100644
--- a/conans/test/integration/revisions_test.py
+++ b/conans/test/integration/revisions_test.py
@@ -1170,6 +1170,20 @@ class SearchingPackagesWithRevisions(unittest.TestCase):
                  assert_error=True)
         self.assertIn("The remote doesn't support revisions", c_v1.out)
 
+    def search_revisions_regular_results_test(self):
+        """If I upload several revisions to a server, we can list the times"""
+        server = TestServer()
+        servers = OrderedDict([("default", server)])
+        c_v2 = TurboTestClient(revisions_enabled=True, servers=servers)
+        pref = c_v2.create(self.ref)
+        c_v2.upload_all(self.ref)
+        pref_rev = pref.copy_with_revs(pref.ref.revision, None)
+
+        c_v2.run("search {} --revisions -r default".format(pref_rev.full_repr()))
+        # I don't want to mock here because I want to run this test against Artifactory
+        self.assertIn("83c38d3b4e5f1b8450434436eec31b00 (", c_v2.out)
+        self.assertIn(" UTC)", c_v2.out)
+
 
 @unittest.skipUnless(get_env("TESTING_REVISIONS_ENABLED", False), "Only revisions")
 class UploadPackagesWithRevisions(unittest.TestCase):
diff --git a/conans/test/unittests/client/util/time_test.py b/conans/test/unittests/client/util/time_test.py
index 3d4fa5d7a..d72134786 100644
--- a/conans/test/unittests/client/util/time_test.py
+++ b/conans/test/unittests/client/util/time_test.py
@@ -15,6 +15,12 @@ class TimeTest(unittest.TestCase):
         expected = datetime.datetime(year=2019, month=1, day=10, hour=16, minute=34, second=59)
         self.assertEquals(dt, expected)
 
+        artifactory_ret = '2019-02-20T13:54:47.543+0000'
+        dt = from_iso8601_to_datetime(artifactory_ret)
+        expected = datetime.datetime(year=2019, month=2, day=20, hour=13, minute=54, second=47,
+                                     microsecond=543000)
+        self.assertEquals(dt, expected)
+
     def validation_test(self):
         self.assertFalse(valid_iso8601("1547138099"))
         self.assertTrue(valid_iso8601("2019-01-10T16:34:59Z"))
diff --git a/conans/test/unittests/util/env_reader.py b/conans/test/unittests/util/env_reader.py
new file mode 100644
index 000000000..02f273e1c
--- /dev/null
+++ b/conans/test/unittests/util/env_reader.py
@@ -0,0 +1,28 @@
+# coding=utf-8
+
+import unittest
+from unittest import mock
+
+from conans.util.env_reader import get_env
+
+
+class GetEnvTest(unittest.TestCase):
+    environment = {"EMPTY_LIST": "",
+                   "LIST": "a,b,c,d"}
+
+    def test_environment(self):
+        """ Ensure that we are using os.environment if no argument is passed """
+        with mock.patch("os.environ.get", return_value="zzz"):
+            a = get_env("whatever", default=None)
+        self.assertEqual(a, "zzz")
+
+    def test_list_empty(self):
+        r = get_env("EMPTY_LIST", default=list(), environment=self.environment)
+        self.assertEqual(r, [])
+
+        r = get_env("NON-EXISTING-LIST", default=list(), environment=self.environment)
+        self.assertEqual(r, [])
+
+    def test_list(self):
+        r = get_env("LIST", default=list(), environment=self.environment)
+        self.assertEqual(r, ["a", "b", "c", "d"])
diff --git a/conans/test/unittests/util/local_db_test.py b/conans/test/unittests/util/local_db_test.py
index 676f49001..b1aaec9aa 100644
--- a/conans/test/unittests/util/local_db_test.py
+++ b/conans/test/unittests/util/local_db_test.py
@@ -10,10 +10,9 @@ class LocalStoreTest(unittest.TestCase):
     def localdb_test(self):
         tmp_dir = temp_folder()
         db_file = os.path.join(tmp_dir, "dbfile")
-        localdb = LocalDB(db_file)
+        localdb = LocalDB.create(db_file)
 
         # Test write and read login
-        localdb.init()
         user, token = localdb.get_login("myurl1")
         self.assertIsNone(user)
         self.assertIsNone(token)
diff --git a/conans/test/utils/tools.py b/conans/test/utils/tools.py
index 7318f12e4..8fcc489c2 100644
--- a/conans/test/utils/tools.py
+++ b/conans/test/utils/tools.py
@@ -262,7 +262,6 @@ class ArtifactoryServerStore(object):
         if not the_json["revisions"]:
             raise RecipeNotFoundException(ref)
         tmp = [_RevisionEntry(i["revision"], i["time"]) for i in the_json["revisions"]]
-        tmp.reverse()
         return tmp
 
     def get_package_revisions(self, pref):
@@ -274,16 +273,15 @@ class ArtifactoryServerStore(object):
         if not the_json["revisions"]:
             raise PackageNotFoundException(pref)
         tmp = [_RevisionEntry(i["revision"], i["time"]) for i in the_json["revisions"]]
-        tmp.reverse()
         return tmp
 
     def get_last_revision(self, ref):
         revisions = self.get_recipe_revisions(ref)
-        return revisions[-1]
+        return revisions[0]
 
     def get_last_package_revision(self, ref):
         revisions = self.get_package_revisions(ref)
-        return revisions[-1]
+        return revisions[0]
 
     def package_exists(self, pref):
         try: