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

Fix import classes to call runAllInteractive() rather than runAllViaWeb (which in practice currently just calls runAllViaWeb ) #24396

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Fix import classes to call runAllInteractive

Before

Import classes call runAllViaWeb

After

Import classes call runAllInteractive

Technical Details

This is a no-change PR at the moment as it will still call runAsWeb unless the not-yet-viable setting to drive it to the background is used - and we have a gazzillion tests that go through that path.

However, this requires some fixes to instantiate the queue more correctly & stop between-test-leakage on the queue singleton (as was hit by the test with the minor doc-block edit)

Comments

@totten this doesn't address the queue runner screen - but I think with this that is the remaining challenge

@civibot civibot bot added the master label Aug 27, 2022
@civibot
Copy link

civibot bot commented Aug 27, 2022

(Standard links)

@eileenmcnaughton eileenmcnaughton changed the title Fix import classes to call runAllInteractive Fix import classes to call runAllInteractive() rather than runAllViaWeb (which in practice currently just calls runAllViaWeb ) Aug 27, 2022
@totten
Copy link
Member

totten commented Sep 6, 2022

@eileenmcnaughton I think these changes look pretty reasonable. Agree that there are other things for background processing to work (but that's OK - because it sits behind an "experimental" feature-flag).

I did a local test run in both modes (default/foreground-runner and experimental/background-runner). For background mode, it was cool to see the background worker actually start doing the job (but not unsurprisingly - it hit an error and then stopped).

For the default/normal mode, it still ran cleanly. 👍 Merging.

@totten totten merged commit 9e5cf4f into civicrm:master Sep 6, 2022
@totten totten deleted the run_back branch September 6, 2022 05:36
@eileenmcnaughton
Copy link
Contributor Author

baby steps :-)

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.

2 participants