Skip to content

Commit

Permalink
NO-REF: Remove RedisManager as an ancestor of CoreProcess (#443)
Browse files Browse the repository at this point in the history
  • Loading branch information
kylevillegas93 authored Nov 13, 2024
1 parent 2561911 commit a2f3991
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 53 deletions.
5 changes: 3 additions & 2 deletions processes/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from typing import Optional

from .core import CoreProcess
from managers import SFRRecordManager, KMeansManager, SFRElasticRecordManager, ElasticsearchManager
from managers import SFRRecordManager, KMeansManager, SFRElasticRecordManager, ElasticsearchManager, RedisManager
from model import Record, Work
from logger import createLog

Expand All @@ -26,7 +26,8 @@ def __init__(self, *args):
self.generateEngine()
self.createSession()

self.createRedisClient()
self.redis_manager = RedisManager()
self.redis_manager.createRedisClient()

self.elastic_search_manager = ElasticsearchManager()

Expand Down
4 changes: 2 additions & 2 deletions processes/core.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from managers import DBManager, RabbitMQManager, RedisManager, S3Manager
from managers import DBManager, RabbitMQManager, S3Manager
from model import Record
from static.manager import StaticManager

Expand All @@ -8,7 +8,7 @@
logger = createLog(__name__)


class CoreProcess(DBManager, RabbitMQManager, RedisManager, StaticManager, S3Manager):
class CoreProcess(DBManager, RabbitMQManager, StaticManager, S3Manager):
def __init__(self, process, customFile, ingestPeriod, singleRecord, batchSize=500):
super(CoreProcess, self).__init__()
self.process = process
Expand Down
7 changes: 4 additions & 3 deletions processes/file/covers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import os

from ..core import CoreProcess
from managers import CoverManager
from managers import CoverManager, RedisManager
from model import Edition, Link
from model.postgres.edition import EDITION_LINKS
from logger import createLog
Expand All @@ -17,7 +17,8 @@ def __init__(self, *args):
self.generateEngine()
self.createSession()

self.createRedisClient()
self.redis_manager = RedisManager()
self.redis_manager.createRedisClient()

self.createS3Client()
self.fileBucket = os.environ['FILE_BUCKET']
Expand Down Expand Up @@ -73,7 +74,7 @@ def searchForCover(self, edition):

def getEditionIdentifiers(self, edition):
for iden in edition.identifiers:
if self.checkSetRedis('sfrCovers', iden.identifier, iden.authority, expirationTime=60*60*24*30):
if self.redis_manager.checkSetRedis('sfrCovers', iden.identifier, iden.authority, expirationTime=60*60*24*30):
logger.debug('{} recently queried. Skipping'.format(iden))
continue

Expand Down
15 changes: 8 additions & 7 deletions processes/frbr/classify.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import re

from ..core import CoreProcess
from managers import OCLCCatalogManager
from managers import OCLCCatalogManager, RedisManager
from mappings.oclc_bib import OCLCBibMapping
from model import Record
from logger import createLog
Expand All @@ -24,7 +24,8 @@ def __init__(self, *args):
self.generateEngine()
self.createSession()

self.createRedisClient()
self.redis_manager = RedisManager()
self.redis_manager.createRedisClient()

self.catalog_queue = os.environ['OCLC_QUEUE']
self.catalog_route = os.environ['OCLC_ROUTING_KEY']
Expand Down Expand Up @@ -78,7 +79,7 @@ def classify_records(self, full=False, start_date_time=None):
record.frbr_status = 'complete'
frbrized_records.append(record)

if self.checkIncrementerRedis('oclcCatalog', 'API'):
if self.redis_manager.checkIncrementerRedis('oclcCatalog', 'API'):
logger.warning('Exceeded max requests to OCLC catalog')
break

Expand Down Expand Up @@ -109,7 +110,7 @@ def frbrize_record(self, record: Record):
except Exception:
author = None

if identifier and self.checkSetRedis('classify', identifier, identifier_type):
if identifier and self.redis_manager.checkSetRedis('classify', identifier, identifier_type):
continue

try:
Expand Down Expand Up @@ -157,7 +158,7 @@ def get_oclc_catalog_records(self, identifiers):

oclc_numbers = list(oclc_numbers)

cached_oclc_numbers = self.multiCheckSetRedis('catalog', oclc_numbers, 'oclc')
cached_oclc_numbers = self.redis_manager.multiCheckSetRedis('catalog', oclc_numbers, 'oclc')

for oclc_number, uncached in cached_oclc_numbers:
if not uncached:
Expand All @@ -168,10 +169,10 @@ def get_oclc_catalog_records(self, identifiers):
catalogued_record_count += 1

if catalogued_record_count > 0:
self.setIncrementerRedis('oclcCatalog', 'API', amount=catalogued_record_count)
self.redis_manager.setIncrementerRedis('oclcCatalog', 'API', amount=catalogued_record_count)

def check_if_classify_work_fetched(self, owi_number: int) -> bool:
return self.checkSetRedis('classifyWork', owi_number, 'owi')
return self.redis_manager.checkSetRedis('classifyWork', owi_number, 'owi')

def _get_queryable_identifiers(self, identifiers):
return list(filter(
Expand Down
8 changes: 6 additions & 2 deletions processes/local_development/seed_local_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from ..core import CoreProcess
from logger import createLog
from managers import RedisManager
from mappings.hathitrust import HathiMapping
from processes import CatalogProcess, ClassifyProcess, ClusterProcess

Expand All @@ -16,6 +17,8 @@ class SeedLocalDataProcess(CoreProcess):
def __init__(self, *args):
super(SeedLocalDataProcess, self).__init__(*args[:4])

self.redis_manager = RedisManager()

def runProcess(self):
try:
self.generateEngine()
Expand All @@ -25,8 +28,9 @@ def runProcess(self):

process_args = ['complete'] + ([None] * 4)

self.createRedisClient()
self.clear_cache()

self.redis_manager.createRedisClient()
self.redis_manager.clear_cache()

classify_process = ClassifyProcess(*process_args)
classify_process.runProcess()
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/processes/file/test_cover_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@

class TestCoverProcess:
@pytest.fixture
def testProcess(self):
def testProcess(self, mocker):
class TestCoverProcess(CoverProcess):
def __init__(self, *args):
self.fileBucket = 'test_aws_bucket'
self.batchSize = 3
self.runTime = datetime(1900, 1, 1)
self.redis_manager = mocker.MagicMock()

return TestCoverProcess()

Expand Down Expand Up @@ -167,16 +168,15 @@ def test_searchForCover_missing(self, testProcess, mocker):
mockManager.resizeCoverFile.assert_not_called()

def test_getEditionIdentifiers(self, testProcess, mocker):
mockCheckRedis = mocker.patch.object(CoverProcess, 'checkSetRedis')
mockCheckRedis.side_effect = [True, False]
testProcess.redis_manager.checkSetRedis.side_effect = [True, False]

mockIdentifiers = [mocker.MagicMock(identifier=1, authority='test'), mocker.MagicMock(identifier=2, authority='test')]
mockEdition = mocker.MagicMock(identifiers=mockIdentifiers)

testIdentifiers = list(testProcess.getEditionIdentifiers(mockEdition))

assert testIdentifiers == [(2, 'test')]
mockCheckRedis.assert_has_calls([
testProcess.redis_manager.checkSetRedis.assert_has_calls([
mocker.call('sfrCovers', 1, 'test', expirationTime=2592000),
mocker.call('sfrCovers', 2, 'test', expirationTime=2592000)
])
Expand Down
47 changes: 18 additions & 29 deletions tests/unit/processes/frbr/test_classify_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def __init__(self, *args):
self.catalog_route = os.environ['OCLC_ROUTING_KEY']
self.classified_count = 0
self.oclc_catalog_manager = mocker.MagicMock()
self.redis_manager = mocker.MagicMock()

return TestClassifyProcess('TestProcess', 'testFile', 'testDate')

Expand Down Expand Up @@ -83,8 +84,7 @@ def test_classify_records_not_full(self, test_instance, mocker):
mock_window_query = mocker.patch.object(ClassifyProcess, 'windowedQuery')
mock_window_query.return_value = mock_records

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkIncrementerRedis')
mock_check_redis.side_effect = [False] * 100
test_instance.redis_manager.checkIncrementerRedis.side_effect = [False] * 100

mock_bulk_save_objects = mocker.patch.object(ClassifyProcess, 'bulkSaveObjects')

Expand All @@ -107,8 +107,7 @@ def test_classify_records_custom_range(self, test_instance, mocker):
mock_window_query = mocker.patch.object(ClassifyProcess, 'windowedQuery')
mock_window_query.return_value = mock_records

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkIncrementerRedis')
mock_check_redis.side_effect = [False] * 100
test_instance.redis_manager.checkIncrementerRedis.side_effect = [False] * 100

mock_bulk_save_objects = mocker.patch.object(ClassifyProcess, 'bulkSaveObjects')

Expand All @@ -132,8 +131,7 @@ def test_classify_records_full(self, test_instance, mocker):
mock_window_query = mocker.patch.object(ClassifyProcess, 'windowedQuery')
mock_window_query.return_value = mock_records

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkIncrementerRedis')
mock_check_redis.side_effect = [False] * 50 + [True]
test_instance.redis_manager.checkIncrementerRedis.side_effect = [False] * 50 + [True]

mock_bulk_save_objects = mocker.patch.object(ClassifyProcess, 'bulkSaveObjects')

Expand All @@ -157,8 +155,7 @@ def test_classify_records_full_batch(self, test_instance, mocker):
mock_window_query = mocker.patch.object(ClassifyProcess, 'windowedQuery')
mock_window_query.return_value = mock_records

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkIncrementerRedis')
mock_check_redis.side_effect = [False] * 100
test_instance.redis_manager.checkIncrementerRedis.side_effect = [False] * 100

mock_bulk_save_objects = mocker.patch.object(ClassifyProcess, 'bulkSaveObjects')

Expand All @@ -174,84 +171,76 @@ def test_frbrize_record_success_valid_author(self, test_instance, test_record, m
mock_identifiers = mocker.patch.object(ClassifyProcess, '_get_queryable_identifiers')
mock_identifiers.return_value = ['1|test']

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkSetRedis')
mock_check_redis.return_value = False
test_instance.redis_manager.checkSetRedis.return_value = False

mock_classify_record = mocker.patch.object(ClassifyProcess, 'classify_record_by_metadata')

test_instance.frbrize_record(test_record)

mock_identifiers.assert_called_once_with(test_record.identifiers)
mock_check_redis.assert_called_once_with('classify', '1', 'test')
test_instance.redis_manager.checkSetRedis.assert_called_once_with('classify', '1', 'test')
mock_classify_record.assert_called_once_with('1', 'test', 'Author, Test', 'Test Record')

def test_frbrize_record_success_author_missing(self, test_instance, test_record, mocker):
mock_identifiers = mocker.patch.object(ClassifyProcess, '_get_queryable_identifiers')
mock_identifiers.return_value = ['1|test']

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkSetRedis')
mock_check_redis.return_value = False
test_instance.redis_manager.checkSetRedis.return_value = False

mock_classify_record = mocker.patch.object(ClassifyProcess, 'classify_record_by_metadata')

test_record.authors = []
test_instance.frbrize_record(test_record)

mock_identifiers.assert_called_once_with(test_record.identifiers)
mock_check_redis.assert_called_once_with('classify', '1', 'test')
test_instance.redis_manager.checkSetRedis.assert_called_once_with('classify', '1', 'test')
mock_classify_record.assert_called_once_with('1', 'test', None, 'Test Record')

def test_frbrize_record_identifier_in_redis(self, test_instance, test_record, mocker):
mock_identifiers = mocker.patch.object(ClassifyProcess, '_get_queryable_identifiers')
mock_identifiers.return_value = ['1|test']

mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkSetRedis')
mock_check_redis.return_value = True
test_instance.redis_manager.checkSetRedis.return_value = True

mock_classify_record = mocker.patch.object(ClassifyProcess, 'classify_record_by_metadata')

test_record.authors = []
test_instance.frbrize_record(test_record)

mock_identifiers.assert_called_once_with(test_record.identifiers)
mock_check_redis.assert_called_once_with('classify', '1', 'test')
test_instance.redis_manager.checkSetRedis.assert_called_once_with('classify', '1', 'test')
mock_classify_record.assert_not_called

def test_frbrize_record_identifier_missing(self, test_instance, test_record, mocker):
mock_identifiers = mocker.patch.object(ClassifyProcess, '_get_queryable_identifiers')
mock_identifiers.return_value = []
mock_check_redis = mocker.patch.object(ClassifyProcess, 'checkSetRedis')
mock_classify_record = mocker.patch.object(ClassifyProcess, 'classify_record_by_metadata')

test_instance.frbrize_record(test_record)

mock_identifiers.assert_called_once_with(test_record.identifiers)
mock_check_redis.assert_not_called()
test_instance.redis_manager.checkSetRedis.assert_not_called()
mock_classify_record.assert_called_once_with(None, None, 'Author, Test', 'Test Record')

def test_get_oclc_catalog_records_no_redis_match(self, test_instance, mocker):
mock_check_redis = mocker.patch.object(ClassifyProcess, 'multiCheckSetRedis')
mock_check_redis.return_value = [('2', True)]
test_instance.redis_manager.multiCheckSetRedis.return_value = [('2', True)]
mock_send_message_to_queue = mocker.patch.object(ClassifyProcess, 'sendMessageToQueue')
mock_set_redis = mocker.patch.object(ClassifyProcess, 'setIncrementerRedis')

test_instance.get_oclc_catalog_records(['1|owi', '2|oclc'])

mock_check_redis.assert_called_once_with('catalog', ['2'], 'oclc')
test_instance.redis_manager.multiCheckSetRedis.assert_called_once_with('catalog', ['2'], 'oclc')
mock_send_message_to_queue.assert_called_once_with('test_oclc_queue', 'test_oclc_key', {'oclcNo': '2', 'owiNo': '1'})
mock_set_redis.assert_called_once_with('oclcCatalog', 'API', amount=1)
test_instance.redis_manager.setIncrementerRedis.assert_called_once_with('oclcCatalog', 'API', amount=1)

def test_get_oclc_catalog_records_redis_match(self, test_instance, mocker):
mock_check_redis = mocker.patch.object(ClassifyProcess, 'multiCheckSetRedis')
mock_check_redis.return_value = [(2, False)]
test_instance.redis_manager.multiCheckSetRedis.return_value = [(2, False)]
mock_send_message_to_queue = mocker.patch.object(ClassifyProcess, 'sendMessageToQueue')
mock_set_redis = mocker.patch.object(ClassifyProcess, 'setIncrementerRedis')

test_instance.get_oclc_catalog_records(['1|test', '2|oclc'])

mock_check_redis.assert_called_once_with('catalog', ['2'], 'oclc')
test_instance.redis_manager.multiCheckSetRedis.assert_called_once_with('catalog', ['2'], 'oclc')
mock_send_message_to_queue.assert_not_called()
mock_set_redis.assert_not_called()
test_instance.redis_manager.setIncrementerReids.assert_not_called()

def test_get_queryable_identifiers(self, test_instance):
assert test_instance._get_queryable_identifiers(['1|isbn', '2|test']) == ['1|isbn']
4 changes: 0 additions & 4 deletions tests/unit/processes/test_core_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ def test_coreProcess_initial_values(self, coreInstance):
assert coreInstance.rabbitHost == 'test_rbmq_host'
assert coreInstance.rabbitPort == 'test_rbmq_port'

# Properties set by the RedisManager
assert coreInstance.redisHost == 'test_redis_host'
assert coreInstance.redisPort == 'test_redis_port'

def test_addDCDWToUpdateList_existing(self, coreInstance, mocker):
mockSession = mocker.MagicMock()
mockSession.query().filter().first.return_value = 'existing_record'
Expand Down

0 comments on commit a2f3991

Please sign in to comment.