From 9f573b8fc7bc232ec10a23f4e39ae6ba5bafbebf Mon Sep 17 00:00:00 2001 From: Ilya Shakhat Date: Mon, 10 Apr 2017 09:57:39 +0000 Subject: [PATCH] Revert "Add support for user correction" This reverts commit bac722a749b11ff0d7cd49b67d7b75ac73658023. There are 2 user profiles that mix with each other because of the same email re-used by other person. The proposed change was intended to fix this, but it does not. All it does is update of user profile after record processing, but at that moment records already refer to the wrong person. The correct solution is to somehow restrict usage of email (e.g. by date) or at least restrict mapping of particular user profiles. Change-Id: I1c37bd9778d740d5095f08885bb3cb63a656a604 --- etc/corrections.json | 7 ---- etc/corrections.schema.json | 26 +------------ stackalytics/processor/main.py | 8 ---- stackalytics/processor/runtime_storage.py | 11 ------ stackalytics/processor/user_processor.py | 12 ++---- .../tests/unit/test_user_processor.py | 38 ------------------- 6 files changed, 5 insertions(+), 97 deletions(-) diff --git a/etc/corrections.json b/etc/corrections.json index 4c1c64063..524defad4 100644 --- a/etc/corrections.json +++ b/etc/corrections.json @@ -2995,12 +2995,5 @@ "module": "openstack-manuals", "subject": "Cleanup the common/ directory" } - ], - "user_corrections": [ - { - "correction_comment": "Reset emails (Related-Bug: #1634020)", - "user_id": "zhangyujun", - "emails": ["yujun.zhang@easystack.cn","284517620@qq.com"] - } ] } diff --git a/etc/corrections.schema.json b/etc/corrections.schema.json index b31f23441..d5c351c3b 100644 --- a/etc/corrections.schema.json +++ b/etc/corrections.schema.json @@ -1,7 +1,7 @@ { "$schema": "http://json-schema.org/draft-04/schema#", "type": "object", - "required": ["corrections", "user_corrections"], + "required": ["corrections"], "properties": { "corrections": { "type": "array", @@ -41,28 +41,6 @@ }, "required": ["primary_key", "correction_comment"] } - }, - "user_corrections": { - "type": "array", - "items": { - "type": "object", - "properties": { - "user_id": { - "type": "string", - "pattern": "^[\\S]+$" - }, - "emails": { - "type": "array", - "items": { - "type": "string", - "pattern": "^[a-z\\d_\\.-]+@([a-z\\d\\.-]+\\.)+[a-z]+$" - } - }, - "correction_comment": { - "type": "string" - } - } - } } } -} +} \ No newline at end of file diff --git a/stackalytics/processor/main.py b/stackalytics/processor/main.py index 4a2180f6a..2630d4202 100644 --- a/stackalytics/processor/main.py +++ b/stackalytics/processor/main.py @@ -262,14 +262,6 @@ def apply_corrections(uri, runtime_storage_inst): LOG.warning('Correction misses primary key: %s', c) runtime_storage_inst.apply_corrections(valid_corrections) - valid_user_corrections = [] - for u in corrections['user_corrections']: - if 'user_id' in u: - valid_user_corrections.append(c) - else: - LOG.warning('User correction misses user_id: %s', u) - runtime_storage_inst.apply_user_corrections(valid_user_corrections) - def process_project_list(runtime_storage_inst): module_groups = runtime_storage_inst.get_by_key('module_groups') or {} diff --git a/stackalytics/processor/runtime_storage.py b/stackalytics/processor/runtime_storage.py index 8f541688f..7ea9ffb53 100644 --- a/stackalytics/processor/runtime_storage.py +++ b/stackalytics/processor/runtime_storage.py @@ -19,8 +19,6 @@ import memcache from oslo_log import log as logging import six - -from stackalytics.processor import user_processor from stackalytics.processor import utils @@ -125,15 +123,6 @@ class MemcachedStorage(RuntimeStorage): self.set_by_key(self._get_record_name(record_id), original) self._commit_update(record_id) - def apply_user_corrections(self, user_corrections_iterator): - for user_correction in user_corrections_iterator: - stored_user = user_processor.load_user(self, - user_id=user_correction[ - 'user_id']) - updated_user = user_processor.update_user_profile( - stored_user, user_correction, is_correction=True) - user_processor.store_user(self, updated_user) - def inc_user_count(self): return self.memcached.incr('user:count') diff --git a/stackalytics/processor/user_processor.py b/stackalytics/processor/user_processor.py index 644de7b84..3ac59014e 100644 --- a/stackalytics/processor/user_processor.py +++ b/stackalytics/processor/user_processor.py @@ -91,19 +91,13 @@ def delete_user(runtime_storage_inst, user): runtime_storage_inst.delete_by_key('user:%s' % user['seq']) -def update_user_profile(stored_user, user, is_correction=False): +def update_user_profile(stored_user, user): # update stored_user with user and return it if stored_user: updated_user = copy.deepcopy(stored_user) updated_user.update(user) - if is_correction: - updated_user['emails'] = user.get('emails', - stored_user.get('emails', [])) - updated_user['corrections'] = stored_user.get('corrections', [])\ - + [user.get('correction_comment', '')] - else: - updated_user['emails'] = list(set(stored_user.get('emails', [])) | - set(user.get('emails', []))) + updated_user['emails'] = list(set(stored_user.get('emails', [])) | + set(user.get('emails', []))) else: updated_user = copy.deepcopy(user) updated_user['static'] = True diff --git a/stackalytics/tests/unit/test_user_processor.py b/stackalytics/tests/unit/test_user_processor.py index f54a21f39..6d816f5ef 100644 --- a/stackalytics/tests/unit/test_user_processor.py +++ b/stackalytics/tests/unit/test_user_processor.py @@ -66,44 +66,6 @@ class TestUserProcessor(testtools.TestCase): # static flag must present self.assertTrue(updated_user.get('static')) - def test_update_user_with_correction(self): - user_correction = { - "user_id": "user", - "correction_comment": "Reset emails", - "emails": ["updated@smith.com"] - } - - stored_user = { - "launchpad_id": "user", - "companies": [ - { - "company_name": "Rackspace", - "end_date": "2011-Nov-20" - }, - { - "company_name": "IBM", - "end_date": None - } - ], - "user_name": "Johnny", - "emails": ["obsoleted@smith.com"], - "corrections": ["Old correction"], - "static": True - } - - updated_user = user_processor.update_user_profile(stored_user, - user_correction, - is_correction=True) - - # reset emails from correction - self.assertEqual(set(user_correction['emails']), - set(updated_user['emails'])) - # save correction history - self.assertEqual(updated_user['corrections'], - ["Old correction", "Reset emails"]) - # static flag must present - self.assertTrue(updated_user.get('static')) - def test_update_user_unknown_user(self): user = { "launchpad_id": "user",