From b721d95d877df4becc4820fb147e9b178790e16d Mon Sep 17 00:00:00 2001 From: Ilya Shakhat Date: Fri, 16 Aug 2013 23:27:36 +0400 Subject: [PATCH] Launchpad lib is replaced by direct request via urllib Launchpad lib is used to find user by his email. The same can be done by accessing LP API directly. This operation doesn't require login to LP and works much faster. Closes bug 1213071 Change-Id: I3a6add2a78e2f493c721840a8c8b9041d8e49654 --- requirements.txt | 1 - stackalytics/processor/main.py | 28 +++----- stackalytics/processor/rcs.py | 2 - stackalytics/processor/record_processor.py | 18 ++--- stackalytics/processor/utils.py | 17 +++++ tests/unit/test_record_processor.py | 80 +++++++--------------- 6 files changed, 61 insertions(+), 85 deletions(-) diff --git a/requirements.txt b/requirements.txt index 1de5ecf0e..c74bca11b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,6 @@ d2to1>=0.2.10,<0.3 Flask>=0.9 Flask-Gravatar iso8601 -launchpadlib oslo.config paramiko>=1.8.0 pbr>=0.5.16,<0.6 diff --git a/stackalytics/processor/main.py b/stackalytics/processor/main.py index 77dc1cbe7..ce151bbb2 100644 --- a/stackalytics/processor/main.py +++ b/stackalytics/processor/main.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import json import urllib from oslo.config import cfg @@ -26,6 +25,7 @@ from stackalytics.processor import default_data_processor from stackalytics.processor import rcs from stackalytics.processor import record_processor from stackalytics.processor import runtime_storage +from stackalytics.processor import utils from stackalytics.processor import vcs @@ -147,12 +147,13 @@ def update_repos(runtime_storage_inst): def apply_corrections(uri, runtime_storage_inst): LOG.info('Applying corrections from uri %s', uri) - corrections_fd = urllib.urlopen(uri) - raw = corrections_fd.read() - corrections_fd.close() - corrections = json.loads(raw)['corrections'] + corrections = utils.read_json_from_uri(uri) + if not corrections: + LOG.error('Unable to read corrections from uri: %s', uri) + return + valid_corrections = [] - for c in corrections: + for c in corrections['corrections']: if 'primary_key' in c: valid_corrections.append(c) else: @@ -160,16 +161,6 @@ def apply_corrections(uri, runtime_storage_inst): runtime_storage_inst.apply_corrections(valid_corrections) -def _read_default_data(uri): - try: - fd = urllib.urlopen(uri) - raw = fd.read() - fd.close() - return json.loads(raw) - except Exception as e: - LOG.error('Error while reading config: %s' % e) - - def main(): # init conf and logging conf = cfg.CONF @@ -183,7 +174,10 @@ def main(): runtime_storage_inst = runtime_storage.get_runtime_storage( cfg.CONF.runtime_storage_uri) - default_data = _read_default_data(cfg.CONF.default_data_uri) + default_data = utils.read_json_from_uri(cfg.CONF.default_data_uri) + if not default_data: + LOG.critical('Unable to load default data') + return not 0 default_data_processor.process(runtime_storage_inst, default_data, cfg.CONF.sources_root) diff --git a/stackalytics/processor/rcs.py b/stackalytics/processor/rcs.py index b6dc2530a..3307f33e4 100644 --- a/stackalytics/processor/rcs.py +++ b/stackalytics/processor/rcs.py @@ -77,8 +77,6 @@ class Gerrit(Rcs): def _get_cmd(self, project_organization, module, branch, sort_key, limit=PAGE_LIMIT): - # This command matches project by substring, not strict - # See https://bugs.launchpad.net/stackalytics/+bug/1212647 cmd = ('gerrit query --all-approvals --patch-sets --format JSON ' 'project:\'%(ogn)s/%(module)s\' branch:%(branch)s ' 'limit:%(limit)s' % diff --git a/stackalytics/processor/record_processor.py b/stackalytics/processor/record_processor.py index 2494af5b4..0480098e1 100644 --- a/stackalytics/processor/record_processor.py +++ b/stackalytics/processor/record_processor.py @@ -16,7 +16,6 @@ import bisect import re -from launchpadlib import launchpad from stackalytics.openstack.common import log as logging from stackalytics.processor import normalizer from stackalytics.processor import utils @@ -78,18 +77,15 @@ class RecordProcessor(object): LOG.debug('User email is not valid %s' % email) else: LOG.debug('Lookup user email %s at Launchpad' % email) - lp = launchpad.Launchpad.login_anonymously('stackalytics', - 'production') - try: - lp_profile = lp.people.getByEmail(email=email) - except Exception as error: - LOG.warn('Lookup of email %s failed %s', email, error.message) + uri = ('https://api.launchpad.net/1.0/people/?' + 'ws.op=getByEmail&email=%s' % email) + lp_profile = utils.read_json_from_uri(uri) if not lp_profile: LOG.debug('User with email %s not found', email) return None, None - return lp_profile.name, lp_profile.display_name + return lp_profile['name'], lp_profile['display_name'] def _get_independent(self): return self.domains_index[''] @@ -144,7 +140,8 @@ class RecordProcessor(object): self._update_record_and_user(record) - yield record + if record['company_name'] != '*robots': + yield record def _spawn_review(self, record): # copy everything except pathsets and flatten user data @@ -162,8 +159,7 @@ class RecordProcessor(object): self._update_record_and_user(review) - if record['company_name'] != '*robots': - yield review + yield review def _spawn_marks(self, record): review_id = record['id'] diff --git a/stackalytics/processor/utils.py b/stackalytics/processor/utils.py index 3df47ff85..544a7dfca 100644 --- a/stackalytics/processor/utils.py +++ b/stackalytics/processor/utils.py @@ -14,7 +14,14 @@ # limitations under the License. import datetime +import json import time +import urllib + +from stackalytics.openstack.common import log as logging + + +LOG = logging.getLogger(__name__) def date_to_timestamp(d): @@ -33,3 +40,13 @@ def week_to_date(week): timestamp = week * 7 * 24 * 3600 + 3 * 24 * 3600 return (datetime.datetime.fromtimestamp(timestamp). strftime('%Y-%m-%d %H:%M:%S')) + + +def read_json_from_uri(uri): + try: + fd = urllib.urlopen(uri) + raw = fd.read() + fd.close() + return json.loads(raw) + except Exception as e: + LOG.warn('Error while reading uri: %s' % e) diff --git a/tests/unit/test_record_processor.py b/tests/unit/test_record_processor.py index 19e6ac490..052e91e2a 100644 --- a/tests/unit/test_record_processor.py +++ b/tests/unit/test_record_processor.py @@ -13,9 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from launchpadlib import launchpad import mock -from oslo.config import cfg import testtools from stackalytics.processor import default_data_processor @@ -24,6 +22,9 @@ from stackalytics.processor import runtime_storage from stackalytics.processor import utils +LP_URI = 'https://api.launchpad.net/1.0/people/?ws.op=getByEmail&email=%s' + + class TestRecordProcessor(testtools.TestCase): def setUp(self): super(TestRecordProcessor, self).setUp() @@ -89,13 +90,13 @@ class TestRecordProcessor(testtools.TestCase): self.runtime_storage = p_storage self.commit_processor = record_processor.RecordProcessor(p_storage) - self.launchpad_patch = mock.patch('launchpadlib.launchpad.Launchpad') - self.launchpad_patch.start() - cfg.CONF = mock.MagicMock() + self.read_json_from_uri_patch = mock.patch( + 'stackalytics.processor.utils.read_json_from_uri') + self.read_json = self.read_json_from_uri_patch.start() def tearDown(self): super(TestRecordProcessor, self).tearDown() - self.launchpad_patch.stop() + self.read_json_from_uri_patch.stop() def _generate_commits(self, email='johndoe@gmail.com', date=1999999999): yield { @@ -150,11 +151,9 @@ class TestRecordProcessor(testtools.TestCase): """ email = 'johndoe@nec.co.jp' commit_generator = self._generate_commits(email=email) - lp_mock = mock.MagicMock() - launchpad.Launchpad.login_anonymously = mock.Mock(return_value=lp_mock) - lp_profile = mock.Mock() - lp_profile.name = 'john_doe' - lp_mock.people.getByEmail = mock.Mock(return_value=lp_profile) + launchpad_id = 'john_doe' + self.read_json.return_value = {'name': launchpad_id, + 'display_name': launchpad_id} user = self.user.copy() # tell storage to return existing user self.get_users.return_value = [user] @@ -163,10 +162,10 @@ class TestRecordProcessor(testtools.TestCase): self.runtime_storage.set_by_key.assert_called_once_with('users', mock.ANY) - lp_mock.people.getByEmail.assert_called_once_with(email=email) + self.read_json.assert_called_once_with(LP_URI % email) self.assertIn(email, user['emails']) self.assertEquals('NEC', commit['company_name']) - self.assertEquals('john_doe', commit['launchpad_id']) + self.assertEquals(launchpad_id, commit['launchpad_id']) def test_update_commit_existing_user_new_email_unknown_company(self): """ @@ -175,11 +174,9 @@ class TestRecordProcessor(testtools.TestCase): """ email = 'johndoe@yahoo.com' commit_generator = self._generate_commits(email=email) - lp_mock = mock.MagicMock() - launchpad.Launchpad.login_anonymously = mock.Mock(return_value=lp_mock) - lp_profile = mock.Mock() - lp_profile.name = 'john_doe' - lp_mock.people.getByEmail = mock.Mock(return_value=lp_profile) + launchpad_id = 'john_doe' + self.read_json.return_value = {'name': launchpad_id, + 'display_name': launchpad_id} user = self.user.copy() # tell storage to return existing user self.get_users.return_value = [user] @@ -188,10 +185,10 @@ class TestRecordProcessor(testtools.TestCase): self.runtime_storage.set_by_key.assert_called_once_with('users', mock.ANY) - lp_mock.people.getByEmail.assert_called_once_with(email=email) + self.read_json.assert_called_once_with(LP_URI % email) self.assertIn(email, user['emails']) self.assertEquals('SuperCompany', commit['company_name']) - self.assertEquals('john_doe', commit['launchpad_id']) + self.assertEquals(launchpad_id, commit['launchpad_id']) def test_update_commit_new_user(self): """ @@ -200,19 +197,16 @@ class TestRecordProcessor(testtools.TestCase): """ email = 'smith@nec.com' commit_generator = self._generate_commits(email=email) - lp_mock = mock.MagicMock() - launchpad.Launchpad.login_anonymously = mock.Mock(return_value=lp_mock) - lp_profile = mock.Mock() - lp_profile.name = 'smith' - lp_profile.display_name = 'Smith' - lp_mock.people.getByEmail = mock.Mock(return_value=lp_profile) + launchpad_id = 'smith' + self.read_json.return_value = {'name': launchpad_id, + 'display_name': 'Smith'} self.get_users.return_value = [] commit = list(self.commit_processor.process(commit_generator))[0] - lp_mock.people.getByEmail.assert_called_once_with(email=email) + self.read_json.assert_called_once_with(LP_URI % email) self.assertEquals('NEC', commit['company_name']) - self.assertEquals('smith', commit['launchpad_id']) + self.assertEquals(launchpad_id, commit['launchpad_id']) def test_update_commit_new_user_unknown_to_lb(self): """ @@ -221,32 +215,12 @@ class TestRecordProcessor(testtools.TestCase): """ email = 'inkognito@avs.com' commit_generator = self._generate_commits(email=email) - lp_mock = mock.MagicMock() - launchpad.Launchpad.login_anonymously = mock.Mock(return_value=lp_mock) - lp_mock.people.getByEmail = mock.Mock(return_value=None) + self.read_json.return_value = None self.get_users.return_value = [] commit = list(self.commit_processor.process(commit_generator))[0] - lp_mock.people.getByEmail.assert_called_once_with(email=email) - self.assertEquals('*independent', commit['company_name']) - self.assertEquals(None, commit['launchpad_id']) - - def test_update_commit_new_user_lb_raises_error(self): - """ - LP raises error during getting user info - """ - email = 'smith@avs.com' - commit_generator = self._generate_commits(email=email) - lp_mock = mock.MagicMock() - launchpad.Launchpad.login_anonymously = mock.Mock(return_value=lp_mock) - lp_mock.people.getByEmail = mock.Mock(return_value=None, - side_effect=Exception) - self.get_users.return_value = [] - - commit = list(self.commit_processor.process(commit_generator))[0] - - lp_mock.people.getByEmail.assert_called_once_with(email=email) + self.read_json.assert_called_once_with(LP_URI % email) self.assertEquals('*independent', commit['company_name']) self.assertEquals(None, commit['launchpad_id']) @@ -256,13 +230,11 @@ class TestRecordProcessor(testtools.TestCase): """ email = 'error.root' commit_generator = self._generate_commits(email=email) - lp_mock = mock.MagicMock() - launchpad.Launchpad.login_anonymously = mock.Mock(return_value=lp_mock) - lp_mock.people.getByEmail = mock.Mock(return_value=None) + self.read_json.return_value = None self.get_users.return_value = [] commit = list(self.commit_processor.process(commit_generator))[0] - self.assertEquals(0, lp_mock.people.getByEmail.called) + self.assertEquals(0, self.read_json.called) self.assertEquals('*independent', commit['company_name']) self.assertEquals(None, commit['launchpad_id'])