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: allow update processor to be overridden by existing yml #273

Merged
merged 1 commit into from
Apr 7, 2020

Conversation

adrexia
Copy link
Contributor

@adrexia adrexia commented Feb 10, 2020

One of the 3->4 update commits (6066af5) replaced SearchUpdateProcessor with SearchUpdateImmediateProcessor, but that means that the existing processor yml fails to replace the SearchUpdateImmediateProcessor with SearchUpdateQueuedJobProcessor as I think is intended?

c.f https://github.com/silverstripe/silverstripe-fulltextsearch/blob/3/_config/processor.yml#L19

One of the update commits (6066af5) replaced SearchUpdateProcessor with SearchUpdateImmediateProcessor, but that means that the existing processor yml fails to replace the SearchUpdateImmediateProcessor with SearchUpdateQueuedJobProcessor  as I think is intended?

c.f https://github.com/silverstripe/silverstripe-fulltextsearch/blob/3/_config/processor.yml#L19
Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, will leave for someone else to review instead of merging though

@emteknetnz
Copy link
Member

Thanks again for creating this pull-request.

Before this gets merged, the team at Silverstripe Ltd are going to need to perform some testing on this work, though at this stage we're not sure when this will get prioritised.

If someone else in the community is able to take over testing duties for this one so that this can be merged sooner, then that would be certainly appreciated.

During the testing phase we may make some alterations to the pull-request, though rest-assured, I'll ensure that you still get credited for the work you've done in the release changelog.

@chillu
Copy link
Member

chillu commented Apr 7, 2020

I've tested this locally on cwp-installer:2.5.x-dev and it works as expected. Thanks for digging into this Naomi. As discussed on the Helpdesk ticket, we also need to look at the autoSoftCommit settings.

@chillu chillu merged commit 4be5de7 into silverstripe:3 Apr 7, 2020
@adrexia
Copy link
Contributor Author

adrexia commented Apr 7, 2020

@chillu thanks!

RE: autoSoftCommit - there is discussion on this ticket that needs a resolution/direction: #274

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

Successfully merging this pull request may close these issues.

None yet

4 participants