From e7b9f5977f98ffe1a5c3f5b0dea4e565b1974ef3 Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Sun, 20 Sep 2026 16:33:04 -0400 Subject: [PATCH 1/7] Second, smaller attempt at using search/api-params from server --- mediacloud/api.py | 75 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/mediacloud/api.py b/mediacloud/api.py index a155559..47c4b59 100644 --- a/mediacloud/api.py +++ b/mediacloud/api.py @@ -4,6 +4,7 @@ import warnings from typing import Any, Dict, List, Optional, Union +from requests import Session from requests_ratelimiter import LimiterSession import mediacloud @@ -32,8 +33,10 @@ class BaseApi: # Default rate limit for API requests. Admins with higher rate limits can # override this on their subclass or instance before creating the session. - RATE_LIMIT_PER_MINUTE = 2 + # This is only an upper bound on the value returned by the api_params method. + RATE_LIMIT_PER_MINUTE = 10 + # tests honor MC_API_BASE_URL environment variable BASE_API_URL = "https://search.mediacloud.org/api/" USER_AGENT_STRING = f"mediacloud {VERSION}" @@ -41,13 +44,63 @@ class BaseApi: def __init__(self, auth_token: Optional[str] = None): if not auth_token: raise mediacloud.error.MCException("No api key set - nothing will work without this") - # Specify the auth_token to use for all future requests - self._auth_token = auth_token + self._headers = { + 'Authorization': f'Token {auth_token}', + 'Accept': 'application/json', + "User-Agent": self.USER_AGENT_STRING, + } # better performance to put all HTTP through this one object; - self._session = LimiterSession(per_minute=self.RATE_LIMIT_PER_MINUTE) - self._session.headers.update({'Authorization': f'Token {self._auth_token}'}) - self._session.headers.update({'Accept': 'application/json'}) - self._session.headers.update({"User-Agent": self.USER_AGENT_STRING}) + self._session: Session | None = None # made on demand + + # saved copies of URL and rate limit used to create _session, + self._per_minute = -1 # initially not valid + + def _make_session(self): + """ + make session object on demand, to allow user manipulation of + BASE_API_URL and RATE_LIMIT_PER_MINUTE after instantiation but + before first call. COULD check for changes BASE_API_URL and + RATE_LIMIT_PER_MINUTE after the fact (by saving the values + used to create the current session, but that seems a bit much) + """ + # make temporary Session object for api_params call + self._session = Session() + self._session.headers.update(self._headers) + + try: + raw = self.api_params() + per_minute = self._parse_rate_limit(raw) + except: + per_minute = 2 # old default + + self._session.close() + self._session = LimiterSession(per_minute=per_minute) + self._session.headers.update(self._headers) + + def _parse_rate_limit(self, raw: JSONObj) -> int: + # :return: rate limit in requests per minute + # tries not to crash, and to handle bad data gracefully + pp = raw.get('params', {}) + if VERSION[0] == 'v' and (apc := pp.get('api-python-client')): + try: + apci = [int(x) for x in apc.split('.')] + vers = [int(x) for x in VERSION[1:].split('.')] + # maybe only compare first two parts (ignore fixes)?? + if apci > vers: + warnings.warn( + f"New mediacloud.api library version available: {apc}") + except ValueError: + pass + + # only support per minute: per hour rates would allow LARGE bursts + # django-ratelimit allows 10/5m, but django-smart-ratelimit may not?? + per_minute = self.RATE_LIMIT_PER_MINUTE + if (qr := pp.get('query-rate')) and isinstance(qr, str): + sr = qr.split('/') # split rate + if len(sr) == 2 and sr[0].isdigit() and sr[1] == 'm': + # use RATE_LIMIT_PER_MINUTE as upper bound + per_minute = min(int(sr[0]), per_minute) + return per_minute def user_profile(self) -> JSONObj: # :return: basic info about the current user, including their roles @@ -64,6 +117,10 @@ def _query(self, endpoint: str, params: Optional[Dict] = None, method: str = 'GE """ Centralize making the actual queries here for easy maintenance and testing of HTTP comms """ + if not self._session: + self._make_session() # create on first call + assert self._session # to quiet mypy + endpoint_url = self.BASE_API_URL + endpoint if method == 'GET': r = self._session.get(endpoint_url, params=params, timeout=self.TIMEOUT_SECS) @@ -100,6 +157,10 @@ def _query(self, endpoint: str, params: Optional[Dict] = None, method: str = 'GE return j + def api_params(self) -> JSONObj: + # :return: api parameters from server + return self._query('search/api-params') + class DirectoryApi(BaseApi): From b7b9404b8567160e0be45eec135bfd0e73914ffe Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Sun, 20 Sep 2026 18:29:51 -0400 Subject: [PATCH 2/7] Update README.md Update api-client github URL Discuss updating server API_PYTHON_CLIENT info --- README.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index eb3ea52..6a9a5dd 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ print("India National Collection has {} sources".format(len(sources))) Development ----------- -If you are interested in adding code to this module, first clone [the GitHub repository](https://github.com/c4fcm/MediaCloud-API-Client). +If you are interested in adding code to this module, first clone [the GitHub repository](https://github.com/mediacloud/api-client). ### Installing @@ -97,3 +97,19 @@ If you are interested in adding code to this module, first clone [the GitHub rep 3. Make a brief note in the `CHANGELOG.md` about what changes 4. Commit changes, and tag commit with version number 5. Push to main + +When client improvements are of great benefit (major new +functionality, speed/rate improvements) update web-search +API_PYTHON_CLIENT config/default so that library users see a (once a +run) warning message that a newer version is available. + +Reminder: This library does not currently detect older than expected +versions of the server, so if changes here depend on a newer +web-search server running, delay release of this library until the new +server version is in production, which means a three-step process: + +1. New server in production +2. Release new client +3. Update server API_PYTHON_CLIENT to new client version + +Doing this out of order with any delay may annoy users! From 37bbde9fd1f752409f46bd295df6964967001a07 Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Mon, 21 Sep 2026 11:42:31 -0400 Subject: [PATCH 3/7] update _per_minute variable; fix comment --- mediacloud/api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mediacloud/api.py b/mediacloud/api.py index 47c4b59..80b1793 100644 --- a/mediacloud/api.py +++ b/mediacloud/api.py @@ -52,7 +52,7 @@ def __init__(self, auth_token: Optional[str] = None): # better performance to put all HTTP through this one object; self._session: Session | None = None # made on demand - # saved copies of URL and rate limit used to create _session, + # saved rate limit used to create _session, self._per_minute = -1 # initially not valid def _make_session(self): @@ -76,6 +76,7 @@ def _make_session(self): self._session.close() self._session = LimiterSession(per_minute=per_minute) self._session.headers.update(self._headers) + self._per_minute = per_minute def _parse_rate_limit(self, raw: JSONObj) -> int: # :return: rate limit in requests per minute From 4f96325fd128ce65e92f92dd8b28d610292d8f73 Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Mon, 21 Sep 2026 18:38:49 -0400 Subject: [PATCH 4/7] Make tests run faster fixes https://github.com/mediacloud/api-client/issues/123 * Use admin connection for higher rates * Force higher rates for directory (rates not enforced) * Keep API objects around in search tests to keep rate limiter state * Removed all sleeps! --- mediacloud/test/api_directory_test.py | 13 ++- mediacloud/test/api_search_test.py | 151 ++++++++++++++------------ 2 files changed, 93 insertions(+), 71 deletions(-) diff --git a/mediacloud/test/api_directory_test.py b/mediacloud/test/api_directory_test.py index bc8a5e3..bbeb9fc 100644 --- a/mediacloud/test/api_directory_test.py +++ b/mediacloud/test/api_directory_test.py @@ -5,19 +5,26 @@ from unittest import TestCase import mediacloud.api +from mediacloud.types import JSONObj TEST_COLLECTION_ID = 34412234 # US -National sources TEST_SOURCE_ID = 1095 # cnn.com TEST_FEED_ID = 1 -mediacloud.api.BaseApi.BASE_API_URL = os.getenv("MC_API_BASE_URL", "https://search.mediacloud.org/api/") +class MyDirectoryApi(mediacloud.api.DirectoryApi): + BASE_API_URL = os.getenv("MC_API_BASE_URL", "https://search.mediacloud.org/api/") + RATE_LIMIT_PER_MINUTE = 60 # upper bound + + def _parse_rate_limit(self, raw: JSONObj) -> int: + # ignore data from api-params call + # (rates only enforced for searches anyway!) + return self.RATE_LIMIT_PER_MINUTE class DirectoryTest(TestCase): def setUp(self): self._mc_api_key = os.getenv("MC_API_TOKEN") - self._directory = mediacloud.api.DirectoryApi(self._mc_api_key) - time.sleep(1) + self._directory = MyDirectoryApi(self._mc_api_key) def test_collection_list_search(self): name_search = 'nigeria' diff --git a/mediacloud/test/api_search_test.py b/mediacloud/test/api_search_test.py index 6ba4363..8fb9a20 100644 --- a/mediacloud/test/api_search_test.py +++ b/mediacloud/test/api_search_test.py @@ -16,23 +16,42 @@ # Optionally override the target instance when testing, for staging/dev cases mediacloud.api.BaseApi.BASE_API_URL = os.getenv("MC_API_BASE_URL", "https://search.mediacloud.org/api/") +mediacloud.api.BaseApi.RATE_LIMIT_PER_MINUTE = 60 # upper bound +# EXPERIMENT: make global Api sessions, so that local library rate limiting +# state kept between tests, avoiding the need for explicit sleeps + +_mc_api_key = os.getenv("MC_API_TOKEN") +_search = mediacloud.api.SearchApi(_mc_api_key) +_mc_api_admin_key = os.getenv("MC_API_ADMIN_TOKEN") +_admin_search = mediacloud.api.SearchApi(_mc_api_admin_key) class BaseSearchTest(TestCase): def setUp(self): - self._mc_api_key = os.getenv("MC_API_TOKEN") - self._search = mediacloud.api.SearchApi(self._mc_api_key) - self._mc_api_admin_key = os.getenv("MC_API_ADMIN_TOKEN") - self._admin_search = mediacloud.api.SearchApi(self._mc_api_admin_key) - time.sleep(30) + # EXPERIMENT: use global API instances, without changing tests for now: + self._mc_api_key = _mc_api_key + self._search = _search + self._admin_search = _admin_search + class SearchAttentionTest(BaseSearchTest): def test_story_count(self): - results = self._search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL], source_ids=[AU_BROADCAST_COMPANY]) + results = self._admin_search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], source_ids=[AU_BROADCAST_COMPANY]) + assert 'relevant' in results + assert results['relevant'] > 0 + assert 'total' in results + assert results['total'] > 0 + assert results['relevant'] <= results['total'] + + def test_warning_no_sources(self): + # test that query with no sources/collections give warning + with pytest.warns(UserWarning): + results = self._admin_search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE) + assert 'relevant' in results assert results['relevant'] > 0 assert 'total' in results @@ -40,8 +59,8 @@ def test_story_count(self): assert results['relevant'] <= results['total'] def test_story_count_over_time(self): - results = self._search.story_count_over_time(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) + results = self._admin_search.story_count_over_time(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) == (END_DATE - START_DATE).days + 1 for day in results: assert 'date' in day @@ -61,20 +80,31 @@ def test_story(self): assert 'url' in story assert 'language' in story assert 'publish_date' in story + assert 'text' not in story # regular user + def test_story_admin(self): + story_id = '9f734354744a651e9b99e4fcd93ee9eaee12ed134ba74dcda13b30234f528535' + story = self._admin_search.story(story_id) + assert 'id' in story + assert story['id'] == story_id + assert 'title' in story + assert 'url' in story + assert 'language' in story + assert 'publish_date' in story + assert 'text' in story # admin user class SearchLanguageTest(BaseSearchTest): def test_words(self): # expected to fail for now - results = self._search.words(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL], - limit=10) + results = self._admin_search.words(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL], + limit=10) assert len(results) > 0 def test_languages(self): - results = self._search.languages(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) + results = self._admin_search.languages(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) > 0 assert results[0]['language'] == 'en' last_ratio = 1 @@ -92,8 +122,8 @@ def test_languages(self): class SearchStoriesTest(BaseSearchTest): def test_sources(self): - results = self._search.sources(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) + results = self._admin_search.sources(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) > 0 last_count = 10000000000 for s in results: @@ -107,7 +137,6 @@ def test_story_list_paging(self): results1, next_page_token1 = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) - time.sleep(31) assert len(results1) == 1000 assert next_page_token1 is not None results2, next_page_token2 = self._admin_search.story_list(query="weather", start_date=START_DATE, @@ -141,7 +170,6 @@ def test_story_list_paging_randomizedd(self): results1, next_page_token1 = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, randomized=True, collection_ids=[COLLECTION_US_NATIONAL]) - time.sleep(31) assert len(results1) == 1000 assert next_page_token1 is not None results2, next_page_token2 = self._admin_search.story_list(query="weather", start_date=START_DATE, @@ -162,7 +190,6 @@ def _test_random_sample(sample_size: int): sample_results = self._admin_search.story_sample(query="weather", start_date=START_DATE, limit=sample_size, end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(sample_results) == sample_size # default length - # time.sleep(31) # get regular results list_results, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, page_size=sample_size, end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) @@ -184,7 +211,6 @@ def test_story_list_random_expanded(self): collection_ids=[COLLECTION_US_NATIONAL], randomized=True) for story in page: assert 'text' not in story - time.sleep(25) page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, expanded=True, collection_ids=[COLLECTION_US_NATIONAL], randomized=True) @@ -198,7 +224,6 @@ def test_story_list_expanded(self): collection_ids=[COLLECTION_US_NATIONAL]) for story in page: assert 'text' not in story - time.sleep(25) page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, expanded=True, collection_ids=[COLLECTION_US_NATIONAL]) for story in page: @@ -216,7 +241,6 @@ def test_story_list_sort_order(self): assert indexed_date <= last_date, "indexed_date not in descending order" last_date = indexed_date # asc - time.sleep(31) page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL], sort_order='asc') a_long_time_ago = dt.datetime(2000, 1, 1, 0, 0, 0) @@ -229,10 +253,10 @@ def test_story_list_sort_order(self): def test_search_by_indexed_date(self): # compare results with indexed_date clause to those without it - results1 = self._search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL]) + results1 = self._admin_search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL]) assert results1['total'] > 0 - results2 = self._search.story_count(query="weather and indexed_date:[{} TO {}]".format( + results2 = self._admin_search.story_count(query="weather and indexed_date:[{} TO {}]".format( START_DATE.isoformat(), END_DATE.isoformat()), start_date=START_DATE, end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) @@ -243,8 +267,8 @@ def test_search_by_indexed_date(self): def test_verify_story_time_formats(self): # indexed_date should have time component - page, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL], page_size=100) + page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], page_size=100) for story in page: assert 'publish_date' in story assert isinstance(story['publish_date'], dt.date) @@ -253,19 +277,18 @@ def test_verify_story_time_formats(self): def test_story_list_page_size(self): # test valid number - page, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL], page_size=103) + page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], page_size=103) assert len(page) == 103 def test_source_ids_filter(self): - results = self._search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, - source_ids=[AU_BROADCAST_COMPANY]) + results = self._admin_search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, + source_ids=[AU_BROADCAST_COMPANY]) assert len(results) == 1 assert results[0]['count'] > 0 assert results[0]['source'] == "abc.net.au" - time.sleep(2) - results, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - source_ids=[AU_BROADCAST_COMPANY]) + results, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + source_ids=[AU_BROADCAST_COMPANY]) assert len(results) > 0 for s in results: assert s['media_url'] == "abc.net.au" @@ -276,38 +299,31 @@ def test_collection_ids_filter(self): directory_api = mediacloud.api.DirectoryApi(self._mc_api_key) limit = 1000 response = directory_api.source_list(collection_id=COLLECTION_US_NATIONAL, limit=limit) - time.sleep(2) sources_in_collection = response['results'] assert len(sources_in_collection) > 200 domains = [s['name'] for s in sources_in_collection] assert len(domains) == len(sources_in_collection) # now check sources to see they're all in collection list of domains - time.sleep(2) - results = self._search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL]) + results = self._admin_search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL]) for s in results: assert s['source'] in domains # now check urls for a page of matches and make sure they're all in collection list of domains - time.sleep(2) - results, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL]) + results, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) > 0 for s in results: assert s['media_url'] in domains -class SearchSyntaxTest(TestCase): +class SearchSyntaxTest(BaseSearchTest): START_DATE = dt.date(2024, 1, 1) - END_DATE = dt.date(2024, 1, 30) - - def setUp(self): - self._mc_api_key = os.getenv("MC_API_TOKEN") - self._search = mediacloud.api.SearchApi(self._mc_api_key) + END_DATE = dt.date(2024, 1, 2) def _count_query(self, query: str) -> int: - return self._search.story_count(query=query, start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL])['relevant'] + return self._admin_search.story_count(query=query, start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL])['relevant'] def test_title_search(self): all_results = self._count_query(query="biden") @@ -384,7 +400,7 @@ def test_negation_source(self): assert minus_count == not_count -class SearchErrorHandlingTest(TestCase): +class SearchErrorHandlingTest(BaseSearchTest): # New test cases for how the api handles bad input and errors from the server. START_DATE = dt.date(2024, 1, 1) @@ -392,31 +408,27 @@ class SearchErrorHandlingTest(TestCase): START_DATETIME = dt.datetime(2024, 1, 1) END_DATETIME = dt.datetime(2024, 1, 30) - def setUp(self): - self._mc_api_key = os.getenv("MC_API_TOKEN") - self._search = mediacloud.api.SearchApi(self._mc_api_key) - def test_datetime(self): query = "biden" - result_via_date = self._search.story_count(query=query, start_date=self.START_DATE, end_date=self.END_DATE, + result_via_date = self._admin_search.story_count(query=query, start_date=self.START_DATE, end_date=self.END_DATE, collection_ids=[COLLECTION_US_NATIONAL])['relevant'] with pytest.warns(UserWarning): - result_via_datetime = self._search.story_count(query=query, start_date=self.START_DATETIME, end_date=self.END_DATETIME, + result_via_datetime = self._admin_search.story_count(query=query, start_date=self.START_DATETIME, end_date=self.END_DATETIME, collection_ids=[COLLECTION_US_NATIONAL])['relevant'] assert result_via_date == result_via_datetime def test_warnings(self): - with patch.object(self._search, "_query", return_value={"count": {}}): + with patch.object(self._admin_search, "_query", return_value={"count": {}}): with pytest.warns(UserWarning, match="start_date was passed as datetime"): - self._search.story_count(query="biden", start_date=self.START_DATETIME, end_date=self.END_DATE, + self._admin_search.story_count(query="biden", start_date=self.START_DATETIME, end_date=self.END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) with pytest.warns(UserWarning, match="end_date was passed as datetime"): - self._search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATETIME, + self._admin_search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATETIME, collection_ids=[COLLECTION_US_NATIONAL]) with pytest.warns(UserWarning, match="No sources or collections specified"): - self._search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATE) + self._admin_search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATE) def test_stories_by_source_over_interval_day(self): expected = [{ @@ -427,8 +439,8 @@ def test_stories_by_source_over_interval_day(self): "total_stories": 100, "ratio": 0.1, }] - with patch.object(self._search, "_query", return_value={"source-interval-attention": expected}) as mock_query: - result = self._search.stories_by_source_over_interval( + with patch.object(self._admin_search, "_query", return_value={"source-interval-attention": expected}) as mock_query: + result = self._admin_search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, @@ -449,11 +461,12 @@ def test_stories_by_source_over_interval_week(self): "total_stories": 50, "ratio": 0.1, }] - with patch.object(self._search, "_query", return_value={"source-interval-attention": expected}) as mock_query: - result = self._search.stories_by_source_over_interval( + with patch.object(self._admin_search, "_query", return_value={"source-interval-attention": expected}) as mock_query: + result = self._admin_search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], interval="week", ) assert result == expected @@ -462,25 +475,27 @@ def test_stories_by_source_over_interval_week(self): assert params["interval"] == "week" def test_stories_by_source_over_interval_default_omits_interval(self): - with patch.object(self._search, "_query", return_value={"source-interval-attention": []}) as mock_query: - self._search.stories_by_source_over_interval( + with patch.object(self._admin_search, "_query", return_value={"source-interval-attention": []}) as mock_query: + self._admin_search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], ) _, params = mock_query.call_args.args assert "interval" not in params def test_stories_by_source_over_interval_propagates_api_error(self): - with patch.object(self._search, "_query", side_effect=mediacloud.error.APIResponseError( + with patch.object(self._admin_search, "_query", side_effect=mediacloud.error.APIResponseError( response=type("Resp", (), {"status_code": 400})(), params={"interval": "bad"}, data={"note": "invalid interval"}, )): with pytest.raises(mediacloud.error.APIResponseError): - self._search.stories_by_source_over_interval( + self._admin_search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], interval="bad", ) From 24ca1585882ccd19cb26d4048118a53faf25ee08 Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Mon, 21 Sep 2026 18:50:22 -0400 Subject: [PATCH 5/7] Add some mypy config to pyproject; quiet some mypy func decl complaints --- mediacloud/api.py | 14 +++++++------- pyproject.toml | 11 +++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/mediacloud/api.py b/mediacloud/api.py index 80b1793..ebe1cfb 100644 --- a/mediacloud/api.py +++ b/mediacloud/api.py @@ -55,7 +55,7 @@ def __init__(self, auth_token: Optional[str] = None): # saved rate limit used to create _session, self._per_minute = -1 # initially not valid - def _make_session(self): + def _make_session(self) -> None: """ make session object on demand, to allow user manipulation of BASE_API_URL and RATE_LIMIT_PER_MINUTE after instantiation but @@ -204,11 +204,11 @@ def feed_list(self, source_id: Optional[int] = None, modified_since: Optional[Union[dt.datetime, int, float]] = None, modified_before: Optional[Union[dt.datetime, int, float]] = None, limit: Optional[int] = 0, offset: Optional[int] = 0, return_details: bool = False) -> JSONObj: - params: Dict[Any, Any] = dict(limit=limit, offset=offset) + params: Dict[str, Any] = dict(limit=limit, offset=offset) if source_id: params['source_id'] = source_id - def epoch_param(t, param): + def epoch_param(t: Union[dt.datetime, int, float], param: str) -> None: if t is None: return # parameter not set if isinstance(t, dt.datetime): @@ -232,7 +232,7 @@ class SearchApi(BaseApi): def _prep_default_params(self, query: str, start_date: dt.date, end_date: dt.date, collection_ids: Optional[List[int]] = [], source_ids: Optional[List[int]] = [], - platform: Optional[str] = None): + platform: Optional[str] = None) -> Dict[str, Any]: if isinstance(start_date, dt.datetime): start_date = start_date.date() @@ -242,7 +242,7 @@ def _prep_default_params(self, query: str, start_date: dt.date, end_date: dt.dat end_date = end_date.date() warnings.warn("end_date was passed as datetime, but expected as date, and has been recast") - params: Dict[Any, Any] = dict(start=start_date.isoformat(), end=end_date.isoformat(), q=query, + params: Dict[str, Any] = dict(start=start_date.isoformat(), end=end_date.isoformat(), q=query, platform=(platform or self.PROVIDER)) if (len(source_ids) + len(collection_ids)) == 0: @@ -305,7 +305,7 @@ def story_list(self, query: str, start_date: dt.date, end_date: dt.date, collect self._dates_str2objects(results['stories']) return results['stories'], results['pagination_token'] - def _dates_str2objects(self, stories: List[Story]): + def _dates_str2objects(self, stories: List[Story]) -> None: # _in place_ translation from ES date str to python data/datetime objects to save memory for s in stories: s['publish_date'] = dt.date.fromisoformat(s['publish_date'][:10]) if s['publish_date'] else None @@ -313,7 +313,7 @@ def _dates_str2objects(self, stories: List[Story]): def story_sample(self, query: str, start_date: dt.date, end_date: dt.date, collection_ids: Optional[List[int]] = [], source_ids: Optional[List[int]] = [], platform: Optional[str] = None, - limit: Optional[int] = None, expanded=False) -> List[Story]: + limit: Optional[int] = None, expanded: bool = False) -> List[Story]: params = self._prep_default_params(query, start_date, end_date, collection_ids, source_ids, platform) if limit: params['limit'] = limit diff --git a/pyproject.toml b/pyproject.toml index 35b07fe..79ccf07 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,3 +38,14 @@ test = [ [project.urls] "Homepage" = "https://mediacloud.org" "Bug Tracker" = "https://github.com/mediacloud/api-client/issues" + +[tool.mypy] +# from story-indexer, +# from https://blog.wolt.com/engineering/2021/09/30/professional-grade-mypy-configuration/ +disallow_untyped_defs = true +disallow_any_unimported = true +no_implicit_optional = true +check_untyped_defs = true +warn_return_any = true +warn_unused_ignores = true +show_error_codes = true From 1f30b653e9fcb826eff33df11d5550d0d5f30a4b Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Thu, 24 Sep 2026 00:36:51 -0400 Subject: [PATCH 6/7] mediacloud/test/api_search_test.py: clean up * Restarted with version from main * Kept global API sessions from new version * Added check for MC_API_TEST_FAST (runs in about 20 sec vs 20 minutes) (by modifying contents of self._search vs using _admin_search) * Made test_story explicitly use _user_search * Kept test_story_admin (explicitly using _admin_search) * Kept addition of collections arg in stories_by_source_over_interval tests --- mediacloud/test/api_search_test.py | 130 ++++++++++++++--------------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/mediacloud/test/api_search_test.py b/mediacloud/test/api_search_test.py index 8fb9a20..0909af9 100644 --- a/mediacloud/test/api_search_test.py +++ b/mediacloud/test/api_search_test.py @@ -18,40 +18,40 @@ mediacloud.api.BaseApi.BASE_API_URL = os.getenv("MC_API_BASE_URL", "https://search.mediacloud.org/api/") mediacloud.api.BaseApi.RATE_LIMIT_PER_MINUTE = 60 # upper bound -# EXPERIMENT: make global Api sessions, so that local library rate limiting -# state kept between tests, avoiding the need for explicit sleeps +# global API sessions, so that local library rate limiting state kept +# between tests, avoiding the need for explicit sleeps to avoid the +# server throwing 429's. This reduces test isolation, but new API +# objects for each test isn't necessarily realistic either. _mc_api_key = os.getenv("MC_API_TOKEN") -_search = mediacloud.api.SearchApi(_mc_api_key) +_user_search = mediacloud.api.SearchApi(_mc_api_key) _mc_api_admin_key = os.getenv("MC_API_ADMIN_TOKEN") _admin_search = mediacloud.api.SearchApi(_mc_api_admin_key) +# MOST tests can be run with either flavor of token; +# allow itchy fingered developers to run tests faster (as admin), +# but default to regular user token. Global for use in skipif decorators. +# On 2026-09-23: runs 19 minutes w/o FAST; 22 seconds with. +_fast = os.getenv('MC_API_TEST_FAST', None) # can be used in .skipif + + class BaseSearchTest(TestCase): def setUp(self): - # EXPERIMENT: use global API instances, without changing tests for now: - self._mc_api_key = _mc_api_key - self._search = _search + # just picking up globals, to avoid mashing the file + if _fast: + self._search = _admin_search + else: + self._search = _user_search self._admin_search = _admin_search - - + self._user_search = _user_search + self._mc_api_key = _mc_api_key # used for DirectoryAPI class SearchAttentionTest(BaseSearchTest): def test_story_count(self): - results = self._admin_search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL], source_ids=[AU_BROADCAST_COMPANY]) - assert 'relevant' in results - assert results['relevant'] > 0 - assert 'total' in results - assert results['total'] > 0 - assert results['relevant'] <= results['total'] - - def test_warning_no_sources(self): - # test that query with no sources/collections give warning - with pytest.warns(UserWarning): - results = self._admin_search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE) - + results = self._search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], source_ids=[AU_BROADCAST_COMPANY]) assert 'relevant' in results assert results['relevant'] > 0 assert 'total' in results @@ -59,8 +59,8 @@ def test_warning_no_sources(self): assert results['relevant'] <= results['total'] def test_story_count_over_time(self): - results = self._admin_search.story_count_over_time(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) + results = self._search.story_count_over_time(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) == (END_DATE - START_DATE).days + 1 for day in results: assert 'date' in day @@ -73,18 +73,18 @@ def test_story_count_over_time(self): def test_story(self): story_id = '9f734354744a651e9b99e4fcd93ee9eaee12ed134ba74dcda13b30234f528535' - story = self._search.story(story_id) + story = self._user_search.story(story_id) # regular user assert 'id' in story assert story['id'] == story_id assert 'title' in story assert 'url' in story assert 'language' in story assert 'publish_date' in story - assert 'text' not in story # regular user + assert 'text' not in story # regular user def test_story_admin(self): story_id = '9f734354744a651e9b99e4fcd93ee9eaee12ed134ba74dcda13b30234f528535' - story = self._admin_search.story(story_id) + story = _admin_search.story(story_id) # NOTE! Admin token. assert 'id' in story assert story['id'] == story_id assert 'title' in story @@ -97,14 +97,14 @@ class SearchLanguageTest(BaseSearchTest): def test_words(self): # expected to fail for now - results = self._admin_search.words(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL], - limit=10) + results = self._search.words(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL], + limit=10) assert len(results) > 0 def test_languages(self): - results = self._admin_search.languages(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) + results = self._search.languages(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) > 0 assert results[0]['language'] == 'en' last_ratio = 1 @@ -122,8 +122,8 @@ def test_languages(self): class SearchStoriesTest(BaseSearchTest): def test_sources(self): - results = self._admin_search.sources(query="weather", start_date=START_DATE, - end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) + results = self._search.sources(query="weather", start_date=START_DATE, + end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) > 0 last_count = 10000000000 for s in results: @@ -253,10 +253,10 @@ def test_story_list_sort_order(self): def test_search_by_indexed_date(self): # compare results with indexed_date clause to those without it - results1 = self._admin_search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL]) + results1 = self._search.story_count(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL]) assert results1['total'] > 0 - results2 = self._admin_search.story_count(query="weather and indexed_date:[{} TO {}]".format( + results2 = self._search.story_count(query="weather and indexed_date:[{} TO {}]".format( START_DATE.isoformat(), END_DATE.isoformat()), start_date=START_DATE, end_date=END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) @@ -267,8 +267,8 @@ def test_search_by_indexed_date(self): def test_verify_story_time_formats(self): # indexed_date should have time component - page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL], page_size=100) + page, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], page_size=100) for story in page: assert 'publish_date' in story assert isinstance(story['publish_date'], dt.date) @@ -277,18 +277,18 @@ def test_verify_story_time_formats(self): def test_story_list_page_size(self): # test valid number - page, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL], page_size=103) + page, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL], page_size=103) assert len(page) == 103 def test_source_ids_filter(self): - results = self._admin_search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, - source_ids=[AU_BROADCAST_COMPANY]) + results = self._search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, + source_ids=[AU_BROADCAST_COMPANY]) assert len(results) == 1 assert results[0]['count'] > 0 assert results[0]['source'] == "abc.net.au" - results, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - source_ids=[AU_BROADCAST_COMPANY]) + results, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + source_ids=[AU_BROADCAST_COMPANY]) assert len(results) > 0 for s in results: assert s['media_url'] == "abc.net.au" @@ -304,13 +304,13 @@ def test_collection_ids_filter(self): domains = [s['name'] for s in sources_in_collection] assert len(domains) == len(sources_in_collection) # now check sources to see they're all in collection list of domains - results = self._admin_search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL]) + results = self._search.sources(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL]) for s in results: assert s['source'] in domains # now check urls for a page of matches and make sure they're all in collection list of domains - results, _ = self._admin_search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL]) + results, _ = self._search.story_list(query="weather", start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL]) assert len(results) > 0 for s in results: assert s['media_url'] in domains @@ -319,11 +319,11 @@ def test_collection_ids_filter(self): class SearchSyntaxTest(BaseSearchTest): START_DATE = dt.date(2024, 1, 1) - END_DATE = dt.date(2024, 1, 2) + END_DATE = dt.date(2024, 1, 30) def _count_query(self, query: str) -> int: - return self._admin_search.story_count(query=query, start_date=START_DATE, end_date=END_DATE, - collection_ids=[COLLECTION_US_NATIONAL])['relevant'] + return self._search.story_count(query=query, start_date=START_DATE, end_date=END_DATE, + collection_ids=[COLLECTION_US_NATIONAL])['relevant'] def test_title_search(self): all_results = self._count_query(query="biden") @@ -410,25 +410,25 @@ class SearchErrorHandlingTest(BaseSearchTest): def test_datetime(self): query = "biden" - result_via_date = self._admin_search.story_count(query=query, start_date=self.START_DATE, end_date=self.END_DATE, + result_via_date = self._search.story_count(query=query, start_date=self.START_DATE, end_date=self.END_DATE, collection_ids=[COLLECTION_US_NATIONAL])['relevant'] with pytest.warns(UserWarning): - result_via_datetime = self._admin_search.story_count(query=query, start_date=self.START_DATETIME, end_date=self.END_DATETIME, + result_via_datetime = self._search.story_count(query=query, start_date=self.START_DATETIME, end_date=self.END_DATETIME, collection_ids=[COLLECTION_US_NATIONAL])['relevant'] assert result_via_date == result_via_datetime def test_warnings(self): - with patch.object(self._admin_search, "_query", return_value={"count": {}}): + with patch.object(self._search, "_query", return_value={"count": {}}): with pytest.warns(UserWarning, match="start_date was passed as datetime"): - self._admin_search.story_count(query="biden", start_date=self.START_DATETIME, end_date=self.END_DATE, + self._search.story_count(query="biden", start_date=self.START_DATETIME, end_date=self.END_DATE, collection_ids=[COLLECTION_US_NATIONAL]) with pytest.warns(UserWarning, match="end_date was passed as datetime"): - self._admin_search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATETIME, + self._search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATETIME, collection_ids=[COLLECTION_US_NATIONAL]) with pytest.warns(UserWarning, match="No sources or collections specified"): - self._admin_search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATE) + self._search.story_count(query="biden", start_date=self.START_DATE, end_date=self.END_DATE) def test_stories_by_source_over_interval_day(self): expected = [{ @@ -439,8 +439,8 @@ def test_stories_by_source_over_interval_day(self): "total_stories": 100, "ratio": 0.1, }] - with patch.object(self._admin_search, "_query", return_value={"source-interval-attention": expected}) as mock_query: - result = self._admin_search.stories_by_source_over_interval( + with patch.object(self._search, "_query", return_value={"source-interval-attention": expected}) as mock_query: + result = self._search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, @@ -461,8 +461,8 @@ def test_stories_by_source_over_interval_week(self): "total_stories": 50, "ratio": 0.1, }] - with patch.object(self._admin_search, "_query", return_value={"source-interval-attention": expected}) as mock_query: - result = self._admin_search.stories_by_source_over_interval( + with patch.object(self._search, "_query", return_value={"source-interval-attention": expected}) as mock_query: + result = self._search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, @@ -475,8 +475,8 @@ def test_stories_by_source_over_interval_week(self): assert params["interval"] == "week" def test_stories_by_source_over_interval_default_omits_interval(self): - with patch.object(self._admin_search, "_query", return_value={"source-interval-attention": []}) as mock_query: - self._admin_search.stories_by_source_over_interval( + with patch.object(self._search, "_query", return_value={"source-interval-attention": []}) as mock_query: + self._search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, @@ -486,13 +486,13 @@ def test_stories_by_source_over_interval_default_omits_interval(self): assert "interval" not in params def test_stories_by_source_over_interval_propagates_api_error(self): - with patch.object(self._admin_search, "_query", side_effect=mediacloud.error.APIResponseError( + with patch.object(self._search, "_query", side_effect=mediacloud.error.APIResponseError( response=type("Resp", (), {"status_code": 400})(), params={"interval": "bad"}, data={"note": "invalid interval"}, )): with pytest.raises(mediacloud.error.APIResponseError): - self._admin_search.stories_by_source_over_interval( + self._search.stories_by_source_over_interval( query="tariff AND Trump", start_date=self.START_DATE, end_date=self.END_DATE, From 494667be2f6d8703871a28a24a24022f9e4e01e3 Mon Sep 17 00:00:00 2001 From: Phil Budne Date: Thu, 24 Sep 2026 14:31:33 -0400 Subject: [PATCH 7/7] Make "pytest --fast" run tests fast. --- conftest.py | 20 +++++++++++++++++++ mediacloud/test/api_search_test.py | 6 ++++-- mediacloud/test/opts.py | 31 ++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 conftest.py create mode 100644 mediacloud/test/opts.py diff --git a/conftest.py b/conftest.py new file mode 100644 index 0000000..8560041 --- /dev/null +++ b/conftest.py @@ -0,0 +1,20 @@ +""" +define global --fast option; needs to be in top-level conftest.py +""" + +import pytest + +# environment variable definitions, private to test directory. +from mediacloud.test.opts import Opts + +def pytest_addoption(parser, *args): + parser.addoption("--fast", action="store_true", + default=Opts.is_set(Opts.MC_API_TEST_FAST), + help="Run tests faster (using admin token)") + +def pytest_configure(config): + """ + called after command line options parsed to copy result back out + to environment for global access using Opts.is_set(Opts.XXX) + """ + Opts.set_bool(Opts.MC_API_TEST_FAST, config.getoption("--fast")) diff --git a/mediacloud/test/api_search_test.py b/mediacloud/test/api_search_test.py index 0909af9..8b682e2 100644 --- a/mediacloud/test/api_search_test.py +++ b/mediacloud/test/api_search_test.py @@ -7,6 +7,7 @@ import pytest import mediacloud.api +from mediacloud.test.opts import Opts COLLECTION_US_NATIONAL = 34412234 AU_BROADCAST_COMPANY = 20775 @@ -32,8 +33,9 @@ # allow itchy fingered developers to run tests faster (as admin), # but default to regular user token. Global for use in skipif decorators. # On 2026-09-23: runs 19 minutes w/o FAST; 22 seconds with. -_fast = os.getenv('MC_API_TEST_FAST', None) # can be used in .skipif - +# Enable by setting MC_API_TEST_FAST environment variable to a non-null value, +# or invoking pytest with --fast +_fast = Opts.is_set(Opts.MC_API_TEST_FAST) class BaseSearchTest(TestCase): diff --git a/mediacloud/test/opts.py b/mediacloud/test/opts.py new file mode 100644 index 0000000..4893a40 --- /dev/null +++ b/mediacloud/test/opts.py @@ -0,0 +1,31 @@ +# global options: +# imported by top-level conftest.py (to get default) +# and by tests (to be able to use in ifskip decorations) + +import os + +class Opts: + """ + string values: pass property names to is_set/set_bool; property + values (which should match) are used as environment variables. + """ + MC_API_TEST_FAST = "MC_API_TEST_FAST" + + @staticmethod + def is_set(opt: str) -> bool: + """ + Non-empty environment values are true. Used both to get command + line option defaults (in top-level conftest.py) AND in tests + to check command line option value. + """ + return bool(os.environ.get(opt)) + + def set_bool(opt: str, val: bool) -> None: + """ + Used to propogate command line options back to environment + (in top-level conftest.py) for use in tests. + """ + if val: + os.environ[opt] = "1" + else: + os.environ.pop(opt, None)