From eca1fd6f5fe0ee81c17c50c310e25810f277e43e Mon Sep 17 00:00:00 2001 From: mcarans Date: Thu, 15 Oct 2026 17:02:58 +1300 Subject: [PATCH] Fix activity paging when limit exceeds HDX maximum and simplify recent additions - Cap activity list limit at 100 when getting all pages, as HDX silently caps it and paging stopped after the first page - Share get_activities body via HDXObject._get_activities - Share download code between Resource.download and download_datastore - Use cls in User.get_current_token_info - Simplify data dictionary merge loop --- src/hdx/data/activity.py | 4 ++- src/hdx/data/dataset.py | 8 +----- src/hdx/data/hdxobject.py | 29 ++++++++++++++++++++- src/hdx/data/location.py | 8 +----- src/hdx/data/organization.py | 8 +----- src/hdx/data/resource.py | 46 +++++++++++++++++++++------------ src/hdx/data/user.py | 12 +++------ tests/hdx/data/test_activity.py | 7 ++++- 8 files changed, 72 insertions(+), 50 deletions(-) diff --git a/src/hdx/data/activity.py b/src/hdx/data/activity.py index e07cdb4..3d0eea7 100644 --- a/src/hdx/data/activity.py +++ b/src/hdx/data/activity.py @@ -129,7 +129,9 @@ def _get_list( result = activity._write_to_hdx(action, data) return [cls(x, configuration=configuration) for x in result] data = dict(data) - limit = data.setdefault("limit", 100) + # HDX silently caps limit at ckan.activity_list_limit_max + limit = min(data.get("limit", 100), 100) + data["limit"] = limit offset = data.get("offset", 0) activities = [] while True: diff --git a/src/hdx/data/dataset.py b/src/hdx/data/dataset.py index 2420901..9116f7b 100755 --- a/src/hdx/data/dataset.py +++ b/src/hdx/data/dataset.py @@ -1448,13 +1448,7 @@ def get_activities(self, all_pages: bool = False, **kwargs: Any) -> list[Activit Returns: List of Activity objects """ - return Activity.get_activities( - "package", - self.data.get("id") or self.data.get("name"), - self.configuration, - all_pages, - **kwargs, - ) + return self._get_activities("package", all_pages, **kwargs) @classmethod def autocomplete( diff --git a/src/hdx/data/hdxobject.py b/src/hdx/data/hdxobject.py index 0925dba..4205154 100755 --- a/src/hdx/data/hdxobject.py +++ b/src/hdx/data/hdxobject.py @@ -9,7 +9,7 @@ from collections.abc import Sequence from os.path import isfile from pathlib import Path -from typing import Any, Optional, Union +from typing import TYPE_CHECKING, Any, Optional, Union from ckanapi.errors import NotFound from hdx.utilities.dictandlist import merge_two_dictionaries @@ -20,6 +20,9 @@ from hdx.api.configuration import Configuration +if TYPE_CHECKING: + from hdx.data.activity import Activity + logger = logging.getLogger(__name__) @@ -376,6 +379,30 @@ def _update_in_hdx( **kwargs, ) + def _get_activities( + self, object_type: str, all_pages: bool = False, **kwargs: Any + ) -> list["Activity"]: + """Helper method to get the activity stream of this object + + Args: + object_type: One of package, organization, group or user + all_pages: Whether to get all pages of results. Defaults to False. + **kwargs: Parameters to pass to Activity.get_activities + + Returns: + List of Activity objects + """ + # Imported here as hdx.data.activity imports this module + from hdx.data.activity import Activity + + return Activity.get_activities( + object_type, + self.data.get("id") or self.data.get("name"), + self.configuration, + all_pages, + **kwargs, + ) + def _write_to_hdx( self, action: str, diff --git a/src/hdx/data/location.py b/src/hdx/data/location.py index 2a5367c..7b4e14d 100644 --- a/src/hdx/data/location.py +++ b/src/hdx/data/location.py @@ -232,13 +232,7 @@ def get_activities(self, all_pages: bool = False, **kwargs: Any) -> list[Activit Returns: List of Activity objects """ - return Activity.get_activities( - "group", - self.data.get("id") or self.data.get("name"), - self.configuration, - all_pages, - **kwargs, - ) + return self._get_activities("group", all_pages, **kwargs) @classmethod def autocomplete( diff --git a/src/hdx/data/organization.py b/src/hdx/data/organization.py index fd01bbe..e8e01ad 100755 --- a/src/hdx/data/organization.py +++ b/src/hdx/data/organization.py @@ -326,13 +326,7 @@ def get_activities(self, all_pages: bool = False, **kwargs: Any) -> list[Activit Returns: List of Activity objects """ - return Activity.get_activities( - "organization", - self.data.get("id") or self.data.get("name"), - self.configuration, - all_pages, - **kwargs, - ) + return self._get_activities("organization", all_pages, **kwargs) @classmethod def autocomplete( diff --git a/src/hdx/data/resource.py b/src/hdx/data/resource.py index 9fd4c41..39a3e6c 100755 --- a/src/hdx/data/resource.py +++ b/src/hdx/data/resource.py @@ -352,10 +352,7 @@ def update_hdx_data_dictionary( merged = [] for column in self.get_hdx_data_dictionary() or []: new_column = new_columns.pop(column.get("field"), None) - if new_column: - merged.append({**column, **new_column}) - else: - merged.append(column) + merged.append({**column, **new_column} if new_column else column) merged.extend(new_columns.values()) self.set_hdx_data_dictionary(merged) self._write_to_hdx( @@ -751,6 +748,30 @@ def search_in_hdx( logger.debug(result) return resources + def _download_file( + self, + url: str, + folder: Path | str | None, + filename: str, + retriever: Retrieve | None, + ) -> Path: + """Helper method to download a file from HDX + + Args: + url: URL to download + folder: Folder to download file to. None means temporary folder. + filename: Filename to use + retriever: Retrieve object to use. None means do not use one. + + Returns: + Path to downloaded file + """ + session = hdx.api.utilities.url_utils.get_ckan_ready_session(self.configuration) + with Download(session=session) as downloader: + if retriever: + downloader = retriever.clone(downloader) + return downloader.download_file(url, folder=folder, filename=filename) + def download( self, folder: Path | str | None = None, retriever: Retrieve | None = None ) -> tuple[str, Path]: @@ -774,12 +795,7 @@ def download( file_format = f".{self.get_format()}" if not filename.endswith(file_format): filename = f"{filename}{file_format}" - session = hdx.api.utilities.url_utils.get_ckan_ready_session(self.configuration) - with Download(session=session) as downloader: - if retriever: - downloader = retriever.clone(downloader) - path = downloader.download_file(url, folder=folder, filename=filename) - return url, path + return url, self._download_file(url, folder, filename, retriever) def get_datastore_dump_url(self, file_format: str = "csv") -> str: """Get the URL from which the resource datastore can be downloaded in @@ -823,19 +839,15 @@ def download_datastore( Returns: (URL downloaded, Path to downloaded file) """ + file_format = file_format.lower() url = self.get_datastore_dump_url(file_format) logger.debug(f"Downloading {url}") if filename is None: filename = self.data["name"] - extension = f".{file_format.lower()}" + extension = f".{file_format}" if not filename.endswith(extension): filename = f"{filename}{extension}" - session = hdx.api.utilities.url_utils.get_ckan_ready_session(self.configuration) - with Download(session=session) as downloader: - if retriever: - downloader = retriever.clone(downloader) - path = downloader.download_file(url, folder=folder, filename=filename) - return url, path + return url, self._download_file(url, folder, filename, retriever) @staticmethod def get_all_resource_ids_in_datastore( diff --git a/src/hdx/data/user.py b/src/hdx/data/user.py index 9053bff..2229edc 100755 --- a/src/hdx/data/user.py +++ b/src/hdx/data/user.py @@ -335,7 +335,7 @@ def get_current_user_organization_dicts( Returns: List of organization dicts in HDX that logged in user is a member of """ - user = User(configuration=configuration) + user = cls(configuration=configuration) try: return user.configuration.call_remoteckan( cls.actions()["listorgs"], {"permission": permission} @@ -471,13 +471,7 @@ def get_activities(self, all_pages: bool = False, **kwargs: Any) -> list[Activit Returns: List of Activity objects """ - return Activity.get_activities( - "user", - self.data.get("id") or self.data.get("name"), - self.configuration, - all_pages, - **kwargs, - ) + return self._get_activities("user", all_pages, **kwargs) @classmethod def get_current_token_info(cls, configuration: Configuration | None = None) -> dict: @@ -491,7 +485,7 @@ def get_current_token_info(cls, configuration: Configuration | None = None) -> d Returns: Dictionary with keys email_hash and token_name """ - user = User(configuration=configuration) + user = cls(configuration=configuration) try: return user.configuration.call_remoteckan(cls.actions()["token_info"], {}) except Exception as e: diff --git a/tests/hdx/data/test_activity.py b/tests/hdx/data/test_activity.py index f4dcfc7..1578bb6 100644 --- a/tests/hdx/data/test_activity.py +++ b/tests/hdx/data/test_activity.py @@ -66,7 +66,7 @@ def post(url, data, **kwargs): ): return mocknotfound(action) offset = datadict.get("offset", 0) - limit = datadict.get("limit", 31) + limit = min(datadict.get("limit", 31), 100) return mockresponse(activities[offset : offset + limit], action) if datadict["id"] != activity_id: return mocknotfound(action) @@ -134,6 +134,11 @@ def test_get_activities_all_pages(self, configuration, post_activity): ) assert [x["id"] for x in result] == [x["id"] for x in activities[150:]] assert [x[1]["offset"] for x in TestActivity.calls] == [150, 200, 250] + TestActivity.calls = [] + result = Activity.get_activities("package", "TEST1", all_pages=True, limit=500) + assert [x["id"] for x in result] == [x["id"] for x in activities] + assert [x[1]["limit"] for x in TestActivity.calls] == [100, 100, 100] + assert [x[1]["offset"] for x in TestActivity.calls] == [0, 100, 200] def test_get_recently_changed_packages(self, configuration, post_activity): result = Activity.get_recently_changed_packages(limit=3)