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

APIv4 Entity.get refactor to be more efficient #20470

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jun 2, 2021

Overview

Makes APIv4 Entity.get more efficient.

Technical Details

APIv4 Entity.get no longer does a file scan when getting one or more entities by name.
Also deprecates the includeCustom param which was redundant with the where clause.
This is the first APIv4 param to be deprecated - added handling to emit a warning when using a deprecated param and hide deprecated params in the APIv4 explorer.

Comments

This PR doesn't go as far as to add caching because there wasn't consensus that it would actually help.

@civibot
Copy link

civibot bot commented Jun 2, 2021

(Standard links)

@civibot civibot bot added the master label Jun 2, 2021
@colemanw
Copy link
Member Author

colemanw commented Jun 2, 2021

@totten this uses the "metadata" cache which by default has a SQL backend. I'm honestly not sure if the SQL query is any better than the filescan. Kinda pointless if not.

@totten
Copy link
Member

totten commented Jun 2, 2021

Not really sure... Some guesspinions for different use-cases:

  • If we're talking about a personal dev-box with one site and huge RAM/CPU resources, then there's probably no difference.
  • If we're talking about shared/cloud hardware and a web-domain that gets sporadic usage, then an intermittent call to civicrm/ajax/api4/Entity/get is probably better if it's served from SQL-cache rather than filescan.
  • If we're talking about a background PHP script that says for ($i=0; $i<100; $i++) { civicrm_api4('Entity','get'); }... then maybe iteration $i=0 is a wash, but iterations $i=1...100 benefit from SqlGroups in-memory frontcache.

This makes Entity.get more efficient - it no longer does a file scan when getting one or more entities by name.
Also deprecates the includeCustom param which was redundant with the where clause.
This is the first APIv4 param to be deprecated - added handling to emit a warning when using a deprecated param
and hide deprecated params in the APIv4 explorer.
@colemanw colemanw changed the title APIv4 Entity.get refactor to use caching APIv4 Entity.get refactor to be more efficient Jun 2, 2021
@colemanw
Copy link
Member Author

colemanw commented Jun 2, 2021

Ok I took out the caching commit, now this PR should be uncontroversial and an easy merge.

@seamuslee001
Copy link
Contributor

This looks fine to me

@seamuslee001 seamuslee001 merged commit e3aaef4 into civicrm:master Jun 3, 2021
@seamuslee001 seamuslee001 deleted the entityGet branch June 3, 2021 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants