Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Ensure list is limited appropriately before evaluating #886

Merged

Conversation

unclecheese
Copy link

@unclecheese unclecheese commented Nov 29, 2018

This prevents a potential memory leak when large numbers of assets are in the database. The get_by_stage call should be limited according to the parameters of the query.

Notes

  • This is not an ideal fix. The reliance on $result['edges'] is due to the resolveList() function returning a data structure that is only suitable for GraphQL. There is a separate PR for GraphQL that abstracts that out. That PR is not a dependency of this fix

  • Because the GraphQL PR is a new API, a separate PR will be made to asset-admin for the next minor release using the new GraphQL methods It's a pretty minor point, and probably not worth quibbling over.

@tractorcow
Copy link
Contributor

I was just sharing the importance of appropriately limiting large datasets. :)

@chillu chillu merged commit 498b8b3 into silverstripe:1.3 Dec 5, 2018
@chillu chillu deleted the pulls/1.3/connection-correction branch December 5, 2018 01:51
@chillu
Copy link
Member

chillu commented Dec 5, 2018

Confirmed this works on a kitchen sink recipe (by checking counts on a large folder before and after)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants