search

silverstripe / silverstripe-fulltextsearch

Adds external full text search engine support to Silverstripe
BSD 3-Clause "New" or "Revised" License
44 stars 83 forks source link

Define getRunAsMemberID #259

Closed wilr closed 4 years ago

wilr commented 5 years ago

On the latest v4.4 CWP pulling down the queued jobs looks like there needs to be an additional definition on each job.

https://github.com/symbiote/silverstripe-queuedjobs/commit/d6160257f74164864454d7766533e946a9d3b4c5

NightJar commented 5 years ago

This is technically a minor release change, could you please re-target the PR to the 3.6* branch?

*which kinda weirdly already exists, although there's no 3.6.0 release yet it seems... :confused:

It would also be super neat if you could squash and give a proper commit message stating why you've made this change (the diff shows what has changed :wink:) i.e. what is the implication of this new method not being present?

If this is a requirement from a dependency, do any constraints also need to be updated in composer.json?

wilr commented 5 years ago

@NightJar I targeted 3.5 since that is currently what CWP ^2.3 pulls in and CWP ^2.3 pulls in the queued job change.

Without this change I can't run any Solr tasks (such as configure or reindex) on ^2.3

robbieaverill commented 5 years ago

@wilr was there a breaking change in FTS? We should try and fix it there if we can

robbieaverill commented 5 years ago

This is the relevant change FYI https://github.com/symbiote/silverstripe-queuedjobs/pull/247/files#diff-bf97514c80202ef07479438c4f389368R213

wilr commented 5 years ago

@robbieaverill only other recommendation rather than updating all the queued jobs (which didn't extend AbstractQueuedJob) ever written is to remove the method from the Interface on queued jobs and just use a hasMethod

robbieaverill commented 5 years ago

Yeah, to be fair I don't think putting it in the interface was a safe change to make in a minor release anyway.

emteknetnz commented 4 years ago

Looks like the breaking change caused by the introduction of a new interface method was resolved by creating a new interface here https://github.com/symbiote/silverstripe-queuedjobs/pull/262/files

The 4.5.0 release does not contain this breaking change https://github.com/symbiote/silverstripe-queuedjobs/blob/626804f02a0060c6182bd5b5509cc4ad058fc813/src/Services/QueuedJob.php

Nor the 4.4.2 release https://github.com/symbiote/silverstripe-queuedjobs/blob/6358ae76cb371a7b77a4d27debf101c846a456e1/src/Services/QueuedJob.php

So I think it's safe to close this pull-request as it does not appear to be required

emteknetnz commented 4 years ago

Closing as we no longer appear to require this pull-request. Thank you for originally submitting it.