diff --git a/changes/222.backport.bugfix b/changes/222.backport.bugfix new file mode 100644 index 00000000000..a8ef7a99301 --- /dev/null +++ b/changes/222.backport.bugfix @@ -0,0 +1 @@ +Backported (d3fff92)[https://github.com/ckan/ckan/commit/d3fff92ebefa1109dfe7cf9c0cad072eaba2bc0b] to fix a flaky issue with resource uploads when updating. diff --git a/changes/222.canada.bugfix b/changes/222.canada.bugfix new file mode 100644 index 00000000000..9998e2e1ad9 --- /dev/null +++ b/changes/222.canada.bugfix @@ -0,0 +1 @@ +Added an Order By to the backref `resources_all` so that the Package ORM object accessing its `resources` property will have the resources in the proper ordering of `position`. This fixes a flaky issue when uploading resources during `package_update` diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index 88bf5ae636c..327856ca132 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -346,7 +346,7 @@ def package_update( changed_resources = original_package['resources'] resource_uploads = [] - for resource in data_dict.get('resources', []): + for resource in changed_resources or []: # file uploads/clearing upload = uploader.get_resource_uploader(resource) @@ -427,7 +427,6 @@ def package_update( resource = next(resiter) upload = next(uploaditer) resource['id'] = pkg.resources[i].id - upload.upload(resource['id'], uploader.get_max_resource_size()) for item in plugins.PluginImplementations(plugins.IPackageController): diff --git a/ckan/model/modification.py b/ckan/model/modification.py index 3e89d346173..9fdf001d109 100644 --- a/ckan/model/modification.py +++ b/ckan/model/modification.py @@ -67,6 +67,8 @@ def notify_observers(self, session: Any, method: Any): def notify(self, entity: Any, operation: Any): for observer in plugins.PluginImplementations( plugins.IDomainObjectModification): + # FIXME: swallowing exceptions hides issues and makes index_package + # difficult to debug try: observer.notify(entity, operation) except SearchIndexError as search_error: diff --git a/ckan/model/resource.py b/ckan/model/resource.py index ec27295ee66..97270bf9cc5 100644 --- a/ckan/model/resource.py +++ b/ckan/model/resource.py @@ -171,6 +171,9 @@ def related_packages(self) -> list[Package]: # formally package_resources_all backref=orm.backref('resources_all', collection_class=ordering_list('position'), + # (canada fork only): proper resource position order for package ORM object + # TODO: upstream contrib!!! + order_by=resource_table.c.position, cascade='all, delete' ), ) diff --git a/ckanext/datapusher/plugin.py b/ckanext/datapusher/plugin.py index c7cc476f2b7..bd6c83623c4 100644 --- a/ckanext/datapusher/plugin.py +++ b/ckanext/datapusher/plugin.py @@ -75,7 +75,7 @@ def after_resource_create( self._submit_to_datapusher(resource_dict) - def after_update( + def after_resource_update( self, context: Context, resource_dict: dict[str, Any]): self._submit_to_datapusher(resource_dict) diff --git a/ckanext/datapusher/tests/test_action.py b/ckanext/datapusher/tests/test_action.py index 0ab187c2168..2280dba160a 100644 --- a/ckanext/datapusher/tests/test_action.py +++ b/ckanext/datapusher/tests/test_action.py @@ -47,7 +47,6 @@ def test_submit(self, monkeypatch): func.assert_called() @pytest.mark.ckan_config("ckan.views.default_views", "") - @pytest.mark.flaky(reruns=2) def test_submit_when_url_changes(self, monkeypatch): dataset = factories.Dataset() func = mock.Mock() diff --git a/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py b/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py index 0b9f7240d0f..0f8abaf5bd8 100644 --- a/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py +++ b/ckanext/example_iresourcecontroller/tests/test_example_iresourcecontroller.py @@ -94,4 +94,4 @@ def test_resource_controller_plugin_show(self): assert plugin.counter["after_resource_update"] == 0, plugin.counter assert plugin.counter["before_resource_delete"] == 0, plugin.counter assert plugin.counter["after_resource_delete"] == 0, plugin.counter - assert plugin.counter["before_resource_show"] == 4, plugin.counter + assert plugin.counter["before_resource_show"] == 3, plugin.counter diff --git a/ckanext/example_iuploader/test/test_plugin.py b/ckanext/example_iuploader/test/test_plugin.py index 4d23d8ea503..3022f00fd95 100644 --- a/ckanext/example_iuploader/test/test_plugin.py +++ b/ckanext/example_iuploader/test/test_plugin.py @@ -59,7 +59,7 @@ def test_resource_download_iuploader_called( "save": "go-metadata", "upload": ("README.rst", CONTENT) }) - assert mock_get_path.call_count == 1 + assert mock_get_path.call_count == 2 assert isinstance(mock_get_path.call_args[0][0], plugin.ResourceUpload) pkg = model.Package.by_name(dataset_name) assert mock_get_path.call_args[0][1] == pkg.resources[0].id