[ckan-dev] Problems with CKAN 2.2.1 (possible BC break?)

Mon Oct 13 15:42:13 UTC 2014

Hi,

This drop of performance is worrying. The only thing I could find that
might be causing this is this commit:

https://github.com/ckan/ckan/commit/5bf5a534e11c4c275de3dec823ae575a6a16e88d

This is calling organization_show on each dataset page load, which
right now may have been getting all datasets associated with this
organization. If the org has a large number of datasets this might
well be the issue.

Can you check if adding these changes to lib/helpers.py (line 1802)
fixes the issue? I've attached the diff so you can apply it easily

Replace
        return logic.get_action('organization_show')({}, {'id': org})

With:
        return logic.get_action('organization_show')({}, {'id': org,
'include_datasets': False})


Thanks,

Adrià


On 13 October 2014 15:28, waldvogel <waldvogel at liip.ch> wrote:
> Hi Adrià
>
> I investigated the issue again (Stefan wrote about what I was running
> into with our upgrade). I tested it again now:
>
> With CKAN 2.2 a dataset page (viewing) is loaded in 500ms-1s. After
> switching to the latest 2.2.1 (with the reverted #1894) it goes up to
> 5-7s for the a dataset page. I could run the harvesting again now but
> that didn't have any effect on this delay. I'll be looking into what
> causes it.
>
> Cheers,
> waldvogel
>
> On Mon, Oct 13, 2014 at 1:58 PM, Adrià Mercader <adria.mercader at okfn.org> wrote:
>> Hi,
>>
>> I've now reverted the #1894 related changes on the release-v2.2.1 branch.
>>
>> @Stefan can you pull the latest version and see if you have any issues.
>>
>> Let us know if you can pinpoint the performance issues to a particular change.
>>
>> Thanks for the heads up,
>>
>> Adrià
>>
>>
>> On 10 October 2014 16:28, Ian Ward <ian at excess.org> wrote:
>>> On Fri, Oct 10, 2014 at 11:25 AM, Adrià Mercader
>>> <adria.mercader at okfn.org> wrote:
>>>> Hi Stefan,
>>>>> It's very well possible that we don't use this as intended, but still I
>>>>> think this violates the "patch releases don't contain backwards incompatible
>>>>> changes". What do you think?
>>>> In hindsight this change might have been to disruptive and the
>>>> potential security issues didn't outweigh the backwards
>>>> incompatibilities. I'd be happy to revert it from the patch release if
>>>> others agree, but would be good to know other core devs opinions?
>>>
>>> Yeah, we shouldn't be changing our API in a patch release. I think we
>>> should revert it.
>>>
>>> Ian
>>> _______________________________________________
>>> ckan-dev mailing list
>>> ckan-dev at lists.okfn.org
>>> https://lists.okfn.org/mailman/listinfo/ckan-dev
>>> Unsubscribe: https://lists.okfn.org/mailman/options/ckan-dev
>> _______________________________________________
>> ckan-dev mailing list
>> ckan-dev at lists.okfn.org
>> https://lists.okfn.org/mailman/listinfo/ckan-dev
>> Unsubscribe: https://lists.okfn.org/mailman/options/ckan-dev
> _______________________________________________
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
> https://lists.okfn.org/mailman/listinfo/ckan-dev
> Unsubscribe: https://lists.okfn.org/mailman/options/ckan-dev
-------------- next part --------------

diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py
index 22b7177..8b7b630 100644
--- a/ckan/lib/helpers.py
+++ b/ckan/lib/helpers.py
@@ -1799,7 +1799,7 @@ def get_organization(org=None):
     if org is None:
         return {}
     try:
-        return logic.get_action('organization_show')({}, {'id': org})
+        return logic.get_action('organization_show')({}, {'id': org, 'include_datasets': False})
     except (NotFound, ValidationError, NotAuthorized):
         return {}
 
