Merge lp:~facundo/magicicada-server/update-client-1440 into lp:magicicada-server

[Facundo Batista]

Updated magicicada client to r1440.

Updated magicicada client to r1440.

Had to remove everything related to DNSs rotation for server selection, and update the way the connection info is passed to the client.

Not related to new client, I also needed to change a little something about Django config: send the complain about "no importable local config" to stderr, so it can be avoided when trying to print settings in Makefile.db

Included several aesthetic changes as I need to do those for `make test` to run properly... in the next branch I'll pin the flake8 revision in the venv so no more of these "spurious fixes" bother us.

[Natalia Bidart (nataliabidart)]

Looks good!

review: Approve

=== modified file 'Makefile.db'
--- Makefile.db	2016-03-08 00:32:28 +0000
+++ Makefile.db	2018-03-20 19:58:45 +0000
@@ -1,7 +1,7 @@
 SHELL_ENV = DJANGO_SETTINGS_MODULE=$(DJANGO_SETTINGS_MODULE) PYTHONPATH=$(PYTHONPATH)
-PGNAME = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print settings.DATABASES['default']['NAME']")
-PGHOST = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print settings.DATABASES['default']['HOST']")
-PGUSER = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print settings.DATABASES['default']['USER']")
+PGNAME = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print settings.DATABASES['default']['NAME']" 2> /dev/null)
+PGHOST = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print settings.DATABASES['default']['HOST']" 2> /dev/null)
+PGUSER = $(shell $(SHELL_ENV) $(PYTHON) -c "from django.conf import settings; print settings.DATABASES['default']['USER']" 2> /dev/null)
 DATA_DIR = $(PGHOST)/data
 LOG_FILE = $(PGHOST)/postgresql.log
 CONF_FILE = $(PGHOST)/postgresql.conf

=== modified file 'config-manager.txt'
--- config-manager.txt	2017-04-22 21:07:36 +0000
+++ config-manager.txt	2018-03-20 19:58:45 +0000
@@ -25,5 +25,5 @@
 ./.sourcecode/dirspec                     ~ubuntuone-control-tower/dirspec/trunk;revno=14
 ./.sourcecode/django                      ~ubuntuone-pqm-team/django/stable;revno=17
 ./.sourcecode/u1sync                      ~facundo/u1sync/opensourcing;revno=10
-./.sourcecode/magicicada-client           ~chicharreros/magicicada-client/trunk;revno=1433
+./.sourcecode/magicicada-client           ~chicharreros/magicicada-client/trunk;revno=1440
 ./.sourcecode/magicicada-protocol         ~chicharreros/magicicada-protocol/trunk;revno=169

=== modified file 'magicicada/server/integtests/test_action_queue.py'
--- magicicada/server/integtests/test_action_queue.py	2016-06-07 19:06:06 +0000
+++ magicicada/server/integtests/test_action_queue.py	2018-03-20 19:58:45 +0000
@@ -35,13 +35,11 @@
     ActionQueue,
     Download,
     Upload,
-    dns_client,
 )
 from ubuntuone.syncdaemon.marker import MDMarker as Marker
 
 from magicicada.server.testing.aq_helpers import (
     FakeFailure,
-    FakeResolver,
     NO_CONTENT_HASH,
     NoCloseStringIO,
     TestBase,
@@ -958,48 +956,3 @@
         d.addBoth(lambda _: self.main.wait_for_nirvana())
         d.addBoth(lambda _: self.eq.push('SYS_USER_DISCONNECT'))
         return d
-
-
-class SRVLookupTest(TestWithDatabase):
-    """ Test for SRV lookup in the ActionQueue. """
-
-    @defer.inlineCallbacks
-    def setUp(self):
-        """
-        Replace the resolver with a FakeResolver
-        """
-        yield super(SRVLookupTest, self).setUp()
-        dns_client.theResolver = FakeResolver()
-
-    @defer.inlineCallbacks
-    def tearDown(self):
-        """
-        By setting the resolver to None, it will be recreated next time a name
-        lookup is done.
-        """
-        yield super(SRVLookupTest, self).tearDown()
-        dns_client.theResolver = None
-
-    def test_SRV_lookup_dev(self):
-        """Test the srv lookup in development mode (localhost:<randomport>)."""
-
-        def checkResult(result):
-            """ Verify that we are correclty doing the lookup """
-            host, port = result
-            self.assertEqual(host, self.aq.host)
-
-        d = self.aq._lookup_srv()
-        d.addCallback(checkResult)
-        return d
-
-    def test_SRV_lookup_prod(self):
-        """ test the srv lookup using a fake resolver. """
-        def checkResult(result):
-            """ Verify that we are correclty doing the lookup """
-            host, port = result
-            self.assertTrue(host in ['fs-1.server.com', 'fs-0.server.com'],
-                            host)
-            self.assertEqual(port, 443)
-        self.aq.dns_srv = '_http._tcp.fs.server.com'
-        d = self.aq._lookup_srv()
-        d.addCallback(checkResult)

=== modified file 'magicicada/server/integtests/test_sync.py'
--- magicicada/server/integtests/test_sync.py	2016-11-07 01:37:46 +0000
+++ magicicada/server/integtests/test_sync.py	2018-03-20 19:58:45 +0000
@@ -113,9 +113,14 @@
         self.share_source_dir = self.mktemp("source/share")
 
         self.patch(hash_queue, "HASHQUEUE_DELAY", 0.1)
+        connection_info = [{
+            'host': "localhost",
+            'port': self.ssl_port,
+            'use_ssl': True,
+            'disable_ssl_verify': True,
+        }]
         self.main = Main(root_dir, shares_dir, data_dir, partials_dir,
-                         "localhost", self.ssl_port, dns_srv=None, ssl=True,
-                         disable_ssl_verify=True)
+                         connection_info=connection_info)
         self.addCleanup(self.main.shutdown)
 
         self.eq = self.main.event_q
@@ -972,8 +977,8 @@
 
         # check locally and in the server
         # it should only be our file in the directory, with content ok
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ["foo"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ["foo"])
         self.assertEqual(open(foo_path).read(), ":)")
         yield self.compare_server()
 
@@ -1130,8 +1135,8 @@
         yield self.main.wait_for_nirvana(.5)
 
         # check that everything is ok: it should only be our file in the dir...
-        l = os.listdir(dir2)
-        self.assertEqual(l, ["test_file"])
+        files = os.listdir(dir2)
+        self.assertEqual(files, ["test_file"])
 
         # ... and our file should have the correct content
         f = open(filepath2)
@@ -1172,8 +1177,8 @@
         def local_check(_):
             """Check that everything is ok"""
             # it should only be our file in the directory
-            l = os.listdir(dir2)
-            self.assertEqual(l, ["test_file"])
+            files = os.listdir(dir2)
+            self.assertEqual(files, ["test_file"])
 
             # our file should have the correct content
             f = open(filepath2)
@@ -1233,8 +1238,8 @@
 
         # wait dust to settle, check locally and in the server
         yield self.main.wait_for_nirvana(.5)
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ["test_file"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ["test_file"])
         yield self.compare_server()
 
     @defer.inlineCallbacks
@@ -1317,8 +1322,8 @@
         yield self.main.wait_for_nirvana(0.5)
 
         # check that everything is ok
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ["test_file"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ["test_file"])
         with open(self.root_dir + "/test_file") as f:
             content = f.read()
         self.assertEqual(content, ":)")
@@ -1474,8 +1479,8 @@
 
         def local_check(_):
             """check that everything is in sync"""
-            l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-            self.assertEqual(l, ["test_file"])
+            files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+            self.assertEqual(files, ["test_file"])
 
             final_data = open(self.root_dir + '/test_file').read()
             self.assertEqual(data, final_data)
@@ -1503,8 +1508,8 @@
 
         def content_check(_):
             """Check that everything is in sync."""
-            l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-            self.assertEqual(l, ["test_dir"])
+            files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+            self.assertEqual(files, ["test_dir"])
             self.assertTrue(stat.S_ISDIR(
                 os.stat(self.root_dir + '/test_dir')[stat.ST_MODE]))
 
@@ -1564,8 +1569,8 @@
         yield self.main.wait_for_nirvana(.5)
 
         # it should only be our file in the directory
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ["test_file"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ["test_file"])
 
         # our file should have the full content
         filesize = os.stat(self.root_dir + "/test_file").st_size
@@ -1610,8 +1615,8 @@
         yield self.main.wait_for_nirvana(.5)
 
         # we should have both files...
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(sorted(l), ["test_file", "test_file.u1conflict"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(sorted(files), ["test_file", "test_file.u1conflict"])
 
         # ...with the respective content
         sanefile = open(test_file).read()
@@ -1631,8 +1636,8 @@
         yield self.main.wait_for_nirvana(.5)
 
         # finally, only one file with right content
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(sorted(l), ["test_file"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(sorted(files), ["test_file"])
         sanefile = open(test_file).read()
         self.assertEqual(sanefile, data_local)
         yield self.compare_server()
@@ -1647,8 +1652,8 @@
         yield self.main.wait_for_nirvana(.5)
 
         # finally, only one file with right content
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(sorted(l), ["test_file", "test_file_new"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(sorted(files), ["test_file", "test_file_new"])
         srvfile = open(test_file).read()
         self.assertEqual(srvfile, data_server)
         newfile = open(test_file + "_new").read()
@@ -1703,8 +1708,8 @@
 
         def no_conflict_check(_):
             """Check the absence of conflict, and the deletion of the node."""
-            l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-            self.assertEqual(l, [])
+            files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+            self.assertEqual(files, [])
 
         d = self.get_client()
         d.addCallback(self.create_and_check)
@@ -1740,8 +1745,8 @@
         self.update_state(None)
         yield self.main.wait_for_nirvana(.5)
 
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(sorted(l), [self.dir_name_conflict])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(sorted(files), [self.dir_name_conflict])
 
 
 class TestConflict2(TestServerBase):
@@ -1757,7 +1762,7 @@
         os.mkdir(self.root_dir + "/test_dir/test_dir")
         try:
             yield self.client.unlink(request.ROOT, req.new_id)
-        except:
+        except Exception:
             # ignore if the unlink fails
             pass
         yield self.main.wait_for_nirvana(0.5)
@@ -1840,8 +1845,8 @@
         yield self.wait_for_nirvana(.5)
 
         # no conflicts, and the server with correct data
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ['testfile'])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ['testfile'])
         self.assertEqual(open(fname).read(), "other content")
         yield self.check()
 
@@ -1879,8 +1884,8 @@
         play_hash()
 
         # check that we don't have conflict, and all is the same with server
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ['tfile'])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ['tfile'])
         self.assertEqual(open(fname).read(), fcontent)
         yield self.check()
 
@@ -1916,8 +1921,8 @@
         yield self.wait_for_nirvana(.5)
 
         # check that we don't have conflict, and all is the same with server
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ['tfile'])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ['tfile'])
         self.assertEqual(open(fname).read(), fcontent)
         yield self.check()
 
@@ -1958,8 +1963,8 @@
         yield self.wait_for_nirvana(.5)
 
         # check that we don't have conflict, and all is the same with server
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(sorted(l), ['tfile'])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(sorted(files), ['tfile'])
         self.assertEqual(open(fname).read(), 'foo')
         yield self.check()
 
@@ -2301,8 +2306,8 @@
         self.aq.get_delta(request.ROOT, 0)
         yield self.main.wait_for_nirvana(0)
 
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ["tfile"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ["tfile"])
         yield self.check()
 
     @defer.inlineCallbacks
@@ -2494,8 +2499,8 @@
         yield wait_download
 
         # check the node is conflicted, both files with corerct content
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(sorted(l), ["test_file", "test_file.u1conflict"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(sorted(files), ["test_file", "test_file.u1conflict"])
         with open(filepath) as f:
             self.assertEqual(f.read(), ":)")
         with open(filepath + ".u1conflict") as f:
@@ -2520,8 +2525,8 @@
         yield self.main.wait_for_nirvana(0.5)
 
         # check that everything is ok
-        l = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
-        self.assertEqual(l, ["test_file"])
+        files = filter_symlinks(self.root_dir, os.listdir(self.root_dir))
+        self.assertEqual(files, ["test_file"])
         with open(filepath) as f:
             self.assertEqual(f.read(), ":)")
 

=== modified file 'magicicada/server/server.py'
--- magicicada/server/server.py	2017-04-22 21:07:36 +0000
+++ magicicada/server/server.py	2018-03-20 19:58:45 +0000
@@ -1698,7 +1698,7 @@
             self._log_exception(exc)
             yield self._send_protocol_error(failure)
             yield self.done()
-        except:
+        except Exception:
             yield self.internal_error(Failure())
 
     def _processMessage(self, message):

=== modified file 'magicicada/server/testing/aq_helpers.py'
--- magicicada/server/testing/aq_helpers.py	2017-04-22 21:07:36 +0000
+++ magicicada/server/testing/aq_helpers.py	2018-03-20 19:58:45 +0000
@@ -165,8 +165,7 @@
 class ReallyFakeMain(main.Main):
     """This main is so fake, it breaks nearly everything."""
 
-    def __init__(self, port, root_dir, data_dir, partials_dir,
-                 dns_srv=None):
+    def __init__(self, port, root_dir, data_dir, partials_dir):
         self.root_dir = root_dir
         self.shares_dir = os.path.join(os.path.dirname(root_dir), 'shares_dir')
         self.shares_dir_link = os.path.join(root_dir, 'shares_link')
@@ -186,8 +185,13 @@
         self.event_q.subscribe(self.vm)
         self.fs.register_eq(self.event_q)
         self.sync = Sync(self)
-        self.action_q = ActionQueue(self.event_q, self, '127.0.0.1', port,
-                                    dns_srv, disable_ssl_verify=True)
+        connection_info = [{
+            'host': '127.0.0.1',
+            'port': port,
+            'use_ssl': False,
+            'disable_ssl_verify': True,
+        }]
+        self.action_q = ActionQueue(self.event_q, self, connection_info)
         self.state_manager = main.StateManager(self, handshake_timeout=30)
         self.state_manager.connection.waiting_timeout = .1
         self.vm.init_root()
@@ -353,7 +357,6 @@
             }
             user = self.make_user(username=username, password=password)
             self.storage_users[username] = user
-        self.dns_srv = None
 
         # override and cleanup user config
         self.old_get_config_files = main.config.get_config_files
@@ -442,8 +445,7 @@
         root_dir = self.mktemp('fake_root_dir')
         data_dir = self.mktemp('fake_data_dir')
         partials = self.mktemp('partials_dir')
-        self.main = ReallyFakeMain(self.port, root_dir,
-                                   data_dir, partials, self.dns_srv)
+        self.main = ReallyFakeMain(self.port, root_dir, data_dir, partials)
         self.state = self.main.state_manager
         self.eq = self.main.event_q
         self.listener = ReallyAttentiveListener()
@@ -765,7 +767,8 @@
         """Nukes the method"""
         self.old = getattr(self.obj, self.meth)
         if func is None:
-            def func(*args, **kwargs): return None
+            def func(*args, **kwargs):
+                return None
         setattr(self.obj, self.meth, func)
         return info
 
@@ -808,7 +811,8 @@
         """Nukes the method"""
         self.old = getattr(self.aq.client, self.meth)
         if func is None:
-            def func(*args, **kwargs): return defer.Deferred
+            def func(*args, **kwargs):
+                return defer.Deferred
         setattr(self.aq.client, self.meth, func)
         return info
 

=== modified file 'magicicada/server/tests/test_shutdown.py'
--- magicicada/server/tests/test_shutdown.py	2016-06-03 00:35:53 +0000
+++ magicicada/server/tests/test_shutdown.py	2018-03-20 19:58:45 +0000
@@ -99,7 +99,7 @@
         try:
             yield client.put_content(request.ROOT, mkfile_req.new_id,
                                      empty_hash, "fake_hash", 1234, 1000, None)
-        except:
+        except Exception:
             client.transport.loseConnection()
 
         ujobs = self.usr0.get_uploadjobs(node_id=mkfile_req.new_id)

=== modified file 'magicicada/settings/__init__.py'
--- magicicada/settings/__init__.py	2017-06-28 01:37:05 +0000
+++ magicicada/settings/__init__.py	2018-03-20 19:58:45 +0000
@@ -32,6 +32,7 @@
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 import logging
 import os
+import sys
 
 from psycopg2.extensions import ISOLATION_LEVEL_REPEATABLE_READ
 
@@ -264,4 +265,4 @@
 try:
     from magicicada.settings.local import *  # noqa
 except ImportError as err:
-    print("ERROR importing local settings:", err)
+    print("ERROR importing local settings:", err, file=sys.stderr)