launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03208
[Merge] lp:~sinzui/launchpad/mlist-sync-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/mlist-sync-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #652007 in Launchpad itself: "scripts/mlist-sync.py should use LaunchpadScript as a base"
https://bugs.launchpad.net/launchpad/+bug/652007
Bug #751897 in Launchpad itself: "mlist-sync test is broken"
https://bugs.launchpad.net/launchpad/+bug/751897
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/mlist-sync-0/+merge/56468
Replace the mailman staging doctest with a unittest.
Launchpad bug: https://bugs.launchpad.net/bugs/751897
Pre-implementation: no one
Test command: ./bin/test -vv -t test_mlist_sync
The mailman/doc/staging.txt test never passes when the mailman layer is
forced to run. Changes to how the testrunner testrunner-appserver may be to
cause of the problem.
--------------------------------------------------------------------
RULES
* Replace the doctest with a unittest that verifies:
* The simple state of a team and a list
* A team with multiple email addresses
* A list without a team.
* ADDENDUM: While constructing the new test. I discovered that the
sync code assumes a team with one email address is for a mailing
list, but this is not true in production. A team can purge a list,
which only affects the Lp db state, the list still exists in mailman,
but it is not active. Teams can have their preferred email address
replaced by an address to a purged list!
* Remove the else clause...the existing loop for multiple addresses
is correct.
QA
None. Even though test showed that the code email address update rules
could update the wrong address, there are no issues in stagings data
at the moment.
select
t.id, t.name
from
person t
join emailaddress e on e.person = t.id
join mailinglist m on m.team = t.id
where
m.status in (9, 10, 11, 12)
and e.email like '%@lists.staging.launchpad.net'
and e.status = 4;
LINT
lib/lp/services/mailman/testing/__init__.py
lib/lp/services/mailman/tests/test_mlist_sync.py
scripts/mlist-sync.py
IMPLEMENTATION
I tried enabling the MailmanLayer in the test runner, but 1 in 3 had a
timeout watching a log :(
I Replaced doc/staging.txt and the testing/sync.py helper with a unittest.
The same three test cases are tested, but the setup and verification is
different. 1. The pseudo production data is created correctly the first
time using a fake config. 2. Storm's flush() and invalidate() are used
instead of rollback() to get a new data from the db.
lib/lp/services/mailman/testing/__init__.py
lib/lp/services/mailman/tests/test_mlist_sync.py
I removed the rule to update a single address for a team because it assumed
the address was for a mailing list. Team will purge email address could
have a non-existent list set as the preferred address.
scripts/mlist-sync.py
--
https://code.launchpad.net/~sinzui/launchpad/mlist-sync-0/+merge/56468
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/mlist-sync-0 into lp:launchpad.
=== removed file 'lib/lp/services/mailman/doc/staging.txt'
--- lib/lp/services/mailman/doc/staging.txt 2010-10-18 22:24:59 +0000
+++ lib/lp/services/mailman/doc/staging.txt 1970-01-01 00:00:00 +0000
@@ -1,159 +0,0 @@
-=================
-Staging migration
-=================
-
-Every once in a while we copy production's database over to staging so that
-testing can be done there on the real database, but without any chance of
-messing up the real site. When this happens, we also need to synchronize
-production's Mailman directories so that the mailing lists all stay consistent
-(well, within the limits of the inherent race conditions). To do this, we use
-scripts/mlist-sync.py.
-
-
-Synchronization
-===============
-
-It's actually fairly difficult to recreate what happens during the staging
-sync, but we can get close. Start off by creating a few mailing lists. When
-the mailing lists are created, messages get sent to the members of the team
-informing them of their new list. We can ignore these.
-
- >>> from canonical.launchpad.ftests import login, logout
- >>> from Mailman import MailList
-
- >>> def makeMailmanList(lp_mailing_list):
- ... mlist = MailList.MailList()
- ... team = lp_mailing_list.team
- ... owner_email = team.teamowner.preferredemail.email
- ... mlist.Create(team.name, owner_email, 'password')
- ... mlist.host_name = 'lists.prod.launchpad.dev'
- ... mlist.web_page_url = 'http://lists.prod.launchpad.dev/mailman/'
- ... mlist.Save()
- ... mlist.addNewMember(owner_email)
- ... mlist.Unlock()
- ... return mlist
-
- >>> login('admin@xxxxxxxxxxxxx')
- >>> team_one, list_one = factory.makeTeamAndMailingList(
- ... 'staging-one', 'owner-one')
- >>> team_two, list_two = factory.makeTeamAndMailingList(
- ... 'staging-two', 'owner-two')
- >>> mlist_one = makeMailmanList(list_one)
- >>> mlist_two = makeMailmanList(list_two)
-
-Give one of the mailing lists a contact address, to further simulate what the
-sync script has to deal with.
-
- >>> from canonical.launchpad.interfaces.emailaddress import (
- ... EmailAddressStatus, IEmailAddressSet)
- >>> from zope.component import getUtility
- >>> email = getUtility(IEmailAddressSet).new(
- ... 'contact@xxxxxxxxxxx', team_two, EmailAddressStatus.VALIDATED)
- >>> team_two.setContactAddress(email)
- >>> logout()
- >>> transaction.commit()
-
-XXX sinzui 2010-10-07 bug=656417: The archive/mbox rsync rules are not tested
-and we need a way to build test versions.
-
-Now we need to simulate the difference between the staging and production
-databases. To do this, we'll use a helper, but first we need to stop Mailman.
-
- >>> from canonical.testing.layers import LayerProcessController
- >>> from lp.services.mailman import runmailman
- >>> runmailman.stop_mailman(
- ... quiet=True, config=LayerProcessController.appserver_config)
- >>> qrunner_watcher.wait_for_shutdown()
-
-In a typical synchronization situation, the production Launchpad database is
-copied to staging at a different time than when the production Mailman data is
-copied. Synchronization does not happen directly to staging's Mailman
-directory, but instead, it's rsync'd to a temporary location on staging, and
-then sync'd by this script from the local temporary location. This ensures
-that the final sync happens pretty quickly and is not dependent on any
-connection between production and staging, but it means that the Mailman data
-may not exactly match the state of the Launchpad database.
-
-For example, it's is possible that a mailing list exists in Launchpad but not
-in the Mailman data if the Mailman data was copied first. Because of the way
-we operationally copy data from production to staging, this generally won't
-happen, so we don't handle it.
-
-The reverse is also possible -- there could be a mailing list in Mailman
-that's not in Launchpad if the Mailman data was copied second (because the
-list was created between the time the Launchpad database was copied and the
-Mailman data was copied). In this case, we just delete the list from Mailman.
-That way if the list is created in Launchpad on staging, the Mailman list will
-also be created.
-
- >>> from lp.services.mailman.testing import sync
- >>> sync_details = sync.prepare_for_sync()
-
-Our simulation now has a staging database that has been copied, but the
-Mailman data has not yet been copied. Let's take a look at what staging's
-Mailman and Launchpad data looks like, before the Mailman data is sync'd.
-Note that fake-team is created by the prepare_for_sync() helper and
-illustrates a mailing list that exists in Mailman but not in Launchpad.
-
- >>> sync.dump_list_info()
- fake-team
- lists.launchpad.dev http://lists.launchpad.dev/mailman/
- No Launchpad team: fake-team
- staging-one
- lists.prod.launchpad.dev http://lists.prod.launchpad.dev/
- staging-one@xxxxxxxxxxxxxxxxxxxxxxxx
- staging-two
- lists.prod.launchpad.dev http://lists.prod.launchpad.dev/
- contact@xxxxxxxxxxx
- staging-two@xxxxxxxxxxxxxxxxxxxxxxxx
-
-Do the Mailman synchronization by calling the script. We must set the
-LPCONFIG environment variable for the script to find the correct
-launchpad.conf file.
-
- >>> import os
- >>> import sys
- >>> from subprocess import Popen, PIPE
- >>> proc = Popen(
- ... ('scripts/mlist-sync.py', '--hostname',
- ... 'lists.prod.launchpad.dev', sync_details.source_dir),
- ... stdout=PIPE, stderr=PIPE,
- ... cwd=LayerProcessController.appserver_config.root,
- ... env=dict(LPCONFIG='testrunner-appserver',
- ... PYTHONPATH=os.pathsep.join(sys.path),
- ... PATH=os.environ.get('PATH')))
- >>> stdout, stderr = proc.communicate()
-
- # Ignore stdout and stderr, but print the return code. stdout should be
- # completely empty and stderr really is uninteresting unless things are
- # broken. It's also problematic because of the way log files are printed
- # (breaking doctest ellipses).
- >>> proc.returncode
- 0
-
- # This is useful for debugging when things go wrong.
- >>> assert proc.returncode == 0, stderr
-
-If the script ran correctly, Mailman should only know about the two mailing
-lists we created above, and not the fake-team list. Also, the host_name and
-web_page_url attributes for these lists should not point to 'prod'.
-
- >>> from canonical.database.sqlbase import rollback
- >>> rollback()
- >>> sync.dump_list_info()
- staging-one
- lists.launchpad.dev http://lists.launchpad.dev/mailman/
- staging-one@xxxxxxxxxxxxxxxxxxx
- staging-two
- lists.launchpad.dev http://lists.launchpad.dev/mailman/
- contact@xxxxxxxxxxx
- staging-two@xxxxxxxxxxxxxxxxxxx
-
-
-Clean up
-========
-
- >>> sync_details.cleanup()
- >>> runmailman.start_mailman(
- ... quiet=True, config=LayerProcessController.appserver_config)
- >>> qrunner_watcher.wait_for_restart()
=== modified file 'lib/lp/services/mailman/testing/__init__.py'
--- lib/lp/services/mailman/testing/__init__.py 2011-01-18 22:37:35 +0000
+++ lib/lp/services/mailman/testing/__init__.py 2011-04-05 21:23:27 +0000
@@ -58,14 +58,17 @@
self.cleanMailmanList(self.mm_list)
def makeMailmanList(self, lp_mailing_list):
- # This utility is based on mailman/tests/TestBase.py.
- mlist = MailList.MailList()
team = lp_mailing_list.team
- self.cleanMailmanList(None, team.name)
owner_email = removeSecurityProxy(team.teamowner).preferredemail.email
- mlist.Create(team.name, owner_email, 'password')
- mlist.host_name = 'lists.launchpad.dev'
- mlist.web_page_url = 'http://lists.launchpad.dev/mailman/'
+ return self.makeMailmanListWithoutTeam(team.name, owner_email)
+
+ def makeMailmanListWithoutTeam(self, list_name, owner_email):
+ # This utility is based on mailman/tests/TestBase.py.
+ self.cleanMailmanList(None, list_name)
+ mlist = MailList.MailList()
+ mlist.Create(list_name, owner_email, 'password')
+ mlist.host_name = mm_cfg.DEFAULT_URL_HOST
+ mlist.web_page_url = 'http://%s/mailman/' % mm_cfg.DEFAULT_URL_HOST
mlist.personalize = 1
mlist.include_rfc2369_headers = False
mlist.use_dollar_strings = True
@@ -84,6 +87,7 @@
'archives/private/%s.mbox',
'archives/public/%s',
'archives/public/%s.mbox',
+ 'mhonarc/%s',
]
for dirtmpl in paths:
list_dir = os.path.join(mm_cfg.VAR_PREFIX, dirtmpl % list_name)
=== removed file 'lib/lp/services/mailman/testing/sync.py'
--- lib/lp/services/mailman/testing/sync.py 2010-12-02 16:13:51 +0000
+++ lib/lp/services/mailman/testing/sync.py 1970-01-01 00:00:00 +0000
@@ -1,144 +0,0 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Helpers for staging sync test."""
-
-__metaclass__ = type
-__all__ = [
- 'SyncDetails',
- 'dump_list_info',
- 'prepare_for_sync',
- ]
-
-
-import os
-import shutil
-import tempfile
-
-from Mailman import mm_cfg
-from Mailman.MailList import MailList
-from Mailman.Utils import list_names
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-
-from canonical.config import config
-from canonical.database.sqlbase import (
- commit,
- flush_database_caches,
- )
-from canonical.launchpad.ftests import (
- login,
- logout,
- )
-from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet
-from lp.registry.interfaces.person import IPersonSet
-
-
-class SyncDetails:
- """Details about the sync that just happened."""
-
- def __init__(self, source_dir, mhonarc_path):
- self.source_dir = source_dir
- self.mhonarc_path = mhonarc_path
-
- def cleanup(self):
- """Clean up all artifacts of the sync."""
- shutil.rmtree(self.source_dir)
- shutil.rmtree(self.mhonarc_path)
-
-
-def prepare_for_sync():
- """Prepare a sync'd directory for mlist-sync.py.
-
- This simulates what happens in the real-world: the production Launchpad
- database is copied to staging, and then the Mailman data is copied to a
- temporary local directory on staging. It is from this temporary location
- that the actual staging Mailman data is sync'd.
-
- Because of this, it's possible that a mailing list will exist in Mailman
- but not in Launchpad's database. We simulate this by creating fake-team
- in Mailman only.
-
- Also, the Mailman data will have some incorrect hostnames that reflect
- production hostnames instead of staging hostnames. We simulate this by
- hacking those production names into the Mailman lists.
-
- The Launchpad database will also have production hostnames in the mailing
- list data it knows about.
-
- Finally, after all this hackery, we copy the current Mailman tree to a
- temporary location. Thus this temporary copy will look like production's
- Mailman database, and the sync will be more realistic.
-
- :return: An object containing the details of the sync. The returned
- object will have a `source_dir` attribute indicating the sync source
- directory, and a `mhonarch_path` attribute indicating the path to the
- fake team's MHonArc archive. The returned object will have a
- `cleanup()` method that must be called at the end of the test.
- :rtype: `SyncDetails`
- """
- # Tweak each of the mailing lists by essentially breaking their host_name
- # and web_page_urls. These will get repaired by the sync script. Do this
- # before we copy so that the production copy will have the busted values.
- # pylint: disable-msg=F0401
- team_names = list_names()
- for list_name in team_names:
- if list_name == mm_cfg.MAILMAN_SITE_LIST:
- continue
- mailing_list = MailList(list_name)
- try:
- mailing_list.host_name = 'lists.prod.launchpad.dev'
- mailing_list.web_page_url = 'http://lists.prod.launchpad.dev'
- mailing_list.Save()
- finally:
- mailing_list.Unlock()
- # Create a mailing list that exists only in Mailman. The sync script will
- # end up deleting this because it represents a race condition between when
- # the production database was copied and when the Mailman data was copied.
- mlist = MailList()
- try:
- mhonarc_path = os.path.join(mm_cfg.VAR_PREFIX, 'mhonarc', 'fake-team')
- mlist.Create('fake-team', mm_cfg.SITE_LIST_OWNER, ' no password ')
- mlist.Save()
- os.makedirs(mhonarc_path)
- finally:
- mlist.Unlock()
- # Create a directory in which to put the simulated production database,
- # then copy our current Mailman stuff to it, lock, stock, and barrel.
- tempdir = tempfile.mkdtemp()
- source_dir = os.path.join(tempdir, 'production')
- shutil.copytree(config.mailman.build_var_dir, source_dir, symlinks=True)
- # Now, we have to mess up the production database by tweaking the email
- # addresses of all the mailing lists.
- login('foo.bar@xxxxxxxxxxxxx')
- email_set = getUtility(IEmailAddressSet)
- for list_name in team_names:
- if list_name == mm_cfg.MAILMAN_SITE_LIST:
- continue
- email = removeSecurityProxy(
- email_set.getByEmail(list_name + '@lists.launchpad.dev'))
- email.email = list_name + '@lists.prod.launchpad.dev'
- logout()
- commit()
- return SyncDetails(source_dir, mhonarc_path)
-
-
-def dump_list_info():
- """Print a bunch of useful information related to sync'ing."""
- # Print interesting information about each mailing list.
- flush_database_caches()
- login('foo.bar@xxxxxxxxxxxxx')
- for list_name in sorted(list_names()):
- if list_name == mm_cfg.MAILMAN_SITE_LIST:
- continue
- mailing_list = MailList(list_name, lock=False)
- print mailing_list.internal_name()
- print ' ', mailing_list.host_name, mailing_list.web_page_url
- team = getUtility(IPersonSet).getByName(list_name)
- if team is None:
- print ' No Launchpad team:', list_name
- else:
- mlist_addresses = getUtility(IEmailAddressSet).getByPerson(team)
- for email in sorted(email.email for email in mlist_addresses):
- print ' ', email
- logout()
=== added file 'lib/lp/services/mailman/tests/test_mlist_sync.py'
--- lib/lp/services/mailman/tests/test_mlist_sync.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/mailman/tests/test_mlist_sync.py 2011-04-05 21:23:27 +0000
@@ -0,0 +1,156 @@
+# Copyright 20010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+"""Test the mlist-sync script."""
+
+__metaclass__ = type
+__all__ = []
+
+from contextlib import contextmanager
+import os
+import shutil
+import sys
+import tempfile
+from textwrap import dedent
+from transaction import commit
+from subprocess import Popen, PIPE
+
+from Mailman import mm_cfg
+from Mailman.MailList import MailList
+from Mailman.Utils import list_names
+
+from canonical.config import config
+from canonical.launchpad.database.emailaddress import EmailAddressSet
+from canonical.launchpad.interfaces.lpstorm import IStore
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.services.mailman.testing import MailmanTestCase
+from lp.testing import person_logged_in
+
+
+@contextmanager
+def production_config(host_name):
+ """Simulate a production Launchpad and mailman config."""
+ config.push('production', dedent("""\
+ [mailman]
+ build_host_name: %s
+ """ % host_name))
+ default_email_host = mm_cfg.DEFAULT_EMAIL_HOST
+ mm_cfg.DEFAULT_EMAIL_HOST = host_name
+ default_url_host = mm_cfg.DEFAULT_URL_HOST
+ mm_cfg.DEFAULT_URL_HOST = host_name
+ try:
+ yield
+ finally:
+ mm_cfg.DEFAULT_URL_HOST = default_url_host
+ mm_cfg.DEFAULT_EMAIL_HOST = default_email_host
+ config.pop('production')
+
+
+class TestMListSync(MailmanTestCase):
+ """Test mlist-sync script."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestMListSync, self).setUp()
+ self.host_name = 'lists.production.launchpad.dev'
+ with production_config(self.host_name):
+ self.team = self.factory.makeTeam(name='team-1')
+ self.mailing_list = self.factory.makeMailingList(
+ self.team, self.team.teamowner)
+ self.mm_list = self.makeMailmanList(self.mailing_list)
+ self.mm_list.Unlock()
+ self.naked_email_address_set = EmailAddressSet()
+
+ def tearDown(self):
+ super(TestMListSync, self).tearDown()
+ self.cleanMailmanList(self.mm_list)
+
+ def setupProductionFiles(self):
+ "Setup a production file structure to sync."
+ tempdir = tempfile.mkdtemp()
+ source_dir = os.path.join(tempdir, 'production')
+ shutil.copytree(
+ config.mailman.build_var_dir, source_dir, symlinks=True)
+ self.addCleanup(shutil.rmtree, source_dir)
+ return source_dir
+
+ def runMListSync(self, source_dir):
+ """Run mlist-sync.py."""
+ store = IStore(self.team)
+ store.flush()
+ commit()
+ store.invalidate()
+ proc = Popen(
+ ('scripts/mlist-sync.py', '--hostname',
+ self.host_name, source_dir),
+ stdout=PIPE, stderr=PIPE,
+ cwd=config.root,
+ env=dict(LPCONFIG=DatabaseFunctionalLayer.appserver_config_name,
+ PYTHONPATH=os.pathsep.join(sys.path),
+ PATH=os.environ.get('PATH')))
+ stdout, stderr = proc.communicate()
+ return proc.returncode, stderr
+
+ def getListInfo(self):
+ """Return a list of 4-tuples of Mailman mailing list info."""
+ list_info = []
+ for list_name in sorted(list_names()):
+ if list_name == mm_cfg.MAILMAN_SITE_LIST:
+ continue
+ mailing_list = MailList(list_name, lock=False)
+ list_address = mailing_list.getListAddress()
+ if self.naked_email_address_set.getByEmail(list_address) is None:
+ email = '%s not found' % list_address
+ else:
+ email = list_address
+ list_info.append(
+ (mailing_list.internal_name(), mailing_list.host_name,
+ mailing_list.web_page_url, email))
+ return list_info
+
+ def test_staging_sync(self):
+ # List is synced with updated URLs and email addresses.
+ source_dir = self.setupProductionFiles()
+ returncode, stderr = self.runMListSync(source_dir)
+ self.assertEqual(0, returncode, stderr)
+ list_summary = [(
+ 'team-1',
+ 'lists.launchpad.dev',
+ 'http://lists.launchpad.dev/mailman/',
+ 'team-1@xxxxxxxxxxxxxxxxxxx')]
+ self.assertEqual(list_summary, self.getListInfo())
+
+ def test_staging_sync_list_without_team(self):
+ # Lists without a team are not synced.
+ with production_config(self.host_name):
+ mlist = self.makeMailmanListWithoutTeam('no-team', 'ex@xxxxxx')
+ mlist.Unlock()
+ os.makedirs(os.path.join(
+ mm_cfg.VAR_PREFIX, 'mhonarc', 'no-team'))
+ self.addCleanup(self.cleanMailmanList, None, 'no-team')
+ source_dir = self.setupProductionFiles()
+ returncode, stderr = self.runMListSync(source_dir)
+ self.assertEqual(0, returncode, stderr)
+ list_summary = [(
+ 'team-1',
+ 'lists.launchpad.dev',
+ 'http://lists.launchpad.dev/mailman/',
+ 'team-1@xxxxxxxxxxxxxxxxxxx')]
+ self.assertEqual(list_summary, self.getListInfo())
+
+ def test_staging_sync_with_team_address(self):
+ # The team's other address is not updated by the sync process.
+ email = self.factory.makeEmail('team-1@xxxxxx', self.team)
+ with production_config(self.host_name):
+ self.team.setContactAddress(email)
+ source_dir = self.setupProductionFiles()
+ returncode, stderr = self.runMListSync(source_dir)
+ self.assertEqual(0, returncode, stderr)
+ list_summary = [(
+ 'team-1',
+ 'lists.launchpad.dev',
+ 'http://lists.launchpad.dev/mailman/',
+ 'team-1@xxxxxxxxxxxxxxxxxxx')]
+ self.assertEqual(list_summary, self.getListInfo())
+ with person_logged_in(self.team.teamowner):
+ self.assertEqual('team-1@xxxxxx', self.team.preferredemail.email)
=== modified file 'scripts/mlist-sync.py'
--- scripts/mlist-sync.py 2010-11-08 12:52:43 +0000
+++ scripts/mlist-sync.py 2011-04-05 21:23:27 +0000
@@ -167,10 +167,8 @@
mlist_addresses = email_address_set.getByPerson(team)
if mlist_addresses.count() == 0:
self.logger.error('No LP email address for: %s', list_name)
- elif mlist_addresses.count() > 1:
- # This team has both a mailing list and a contact address. We
- # only want to change the former, but we need some heuristics
- # to figure out which is which.
+ else:
+ # Teams can have both a mailing list and a contact address.
old_address = '%s@%s' % (list_name, self.options.hostname)
for email_address in mlist_addresses:
if email_address.email == old_address:
@@ -181,12 +179,6 @@
else:
self.logger.error('No change to LP email address for: %s',
list_name)
- else:
- email_address = removeSecurityProxy(mlist_addresses[0])
- old_address = email_address.email
- email_address.email = lp_mailing_list.address
- self.logger.info('%s -> %s',
- old_address, lp_mailing_list.address)
def deleteMailmanList(self, list_name):
"""Delete all Mailman data structures for `list_name`."""
