Don't put API calls in the upgrader

[totten]

n/t

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

In case the log disappears they're all this one in the stock preupgrade messages

civicrm-core/CRM/Upgrade/Incremental/General.php

Line 137 in 6c2c453

$financialAclExtension = civicrm_api3('extension', 'get', ['key' => 'biz.jmaconsulting.financialaclreport', 'sequential' => 1]);

[colemanw]

@totten FYI there was some work done last year to make APIv4 calls upgrade-safe, because it filters values by CRM_Core_DAO::getSupportedFields and that function only returns fields supported by the current version (even during an upgrade state).

[colemanw]

retest this please

[demeritcowboy]

Patch no longer applies. Github says yes, git says no. One of them is lying.

[totten]

@demeritcowboy @colemanw I've rebased, and... it does pass now... though it's slightly deceptive, because
this bit hides some failed assertions (and I don't really have my head around the range of errors that are properly or improperly captured there).

[demeritcowboy]

I'm just wondering if it's too harsh to completely prevent it. For example one of them is just setting a setting, which ok could be a SQL query where you serialize it yourself, but it seems like the risks in using the function is low. Or some of them are ensureOptionValueExists(), which is intended to be used in upgrade scripts, so if that one's dangerous we should update that function.
But I suppose we could always grow the functionThatIsNotSupposedToGrow if it's decided a future given upgrade task could also be excluded.

[demeritcowboy]

@totten What do you want to do with this one? Close for now as being too nuanced to be so strict?

[eileenmcnaughton]

Since the last action here was to propose closing this in June I'm gonna do just that - I've opened a ticket to track it - https://lab.civicrm.org/dev/core/-/issues/4823