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

[Adrià Mercader]

Pull request is here:

https://github.com/ckan/ckan/pull/1978

Core devs, another pair of eyes to review this would be handy :)

Adrià

On 13 October 2014 16:42, Adrià Mercader <adria.mercader at okfn.org> wrote:
> 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