Problem/Motivation

The first implementation of post-processing used events to trigger processing. Since #2423461: Basic post-processing with configurable plugins there is a plugin-based solution that complies with configurable schemas, so the event-based code is deprecated.

Proposed resolution

Remove the event-based processing when the plugin-based processing has all of its features:

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#3 remove_event_based-2441983-3.patch5.22 KBmbovan

Comments

corvus_ch’s picture

I do not see any reason why this should be removed. We now introduced a more structured way of processing with processor plugins. This seems to be the right approach for what we do what to achieve but may not be a match for somebody else. So I suggest that we leave the event based processing inlace just for the reason of options.

So instead of removing it, let's do some clean up. The event based processing now is only triggered by the service and only handled in background. So lets move it to posSave and make it configurable to be executed directly or moved to background processing.

miro_dietiker’s picture

Because what you propose is extra work.

We are not saying that this feature is crap. We just say, our current direction does not use it.
And we are trying to limit the codebase and focus work to speedup. Dropping features for this makes sense, because it is fast, reduces complexity.

Usually, we provide a removal patch and commit it. Then keeping the issue open.
Once the need arises again with a real world application, the diffs / patches can be applied in reverse and someone can spend time in completing the feature.

mbovan’s picture

Component: Metadata Management » Storage
Status: Active » Needs review
StatusFileSize
new5.22 KB

I'm working on CRM Core Matcher changes (Issue Reimplement Matcher with plugin instances support ) and during CRM Core and Collect integration spotted that this class is no longer used.

Providing a patch to remove left-overs. If we decide to switch to approach EventSubscriber implements we can use this patch again to revert the changes.

miro_dietiker’s picture

You just dropped the EventSubscriber. I committed it for now to drop that complexity.

Note that the whole event processing is still in place:

./src/Event/CollectEvent.php: * Contains \Drupal\collect\Event\CollectEvent.
./src/Event/CollectEvent.php:namespace Drupal\collect\Event;
./src/Event/CollectEvent.php:use Symfony\Component\EventDispatcher\Event;
./src/Event/CollectEvent.php: * Event fired by the queue worker for a new Collect submission.
./src/Event/CollectEvent.php:class CollectEvent extends Event {
./src/Event/CollectEvent.php:   * Constructs a CollectEvent object.
./src/Plugin/QueueWorker/SubmissionProcessing.php:use Drupal\collect\Event\CollectEvent;
./src/Plugin/QueueWorker/SubmissionProcessing.php:use Symfony\Component\EventDispatcher\EventDispatcherInterface;
./src/Plugin/QueueWorker/SubmissionProcessing.php:   * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface
./src/Plugin/QueueWorker/SubmissionProcessing.php:  public function __construct(array $configuration, $plugin_id, $plugin_definition, EventDispatcherInterface $event_dispatcher) {
./src/Plugin/QueueWorker/SubmissionProcessing.php:    $this->eventDispatcher->dispatch(CollectEvent::NAME, new CollectEvent($data));
miro_dietiker’s picture

Status: Needs review » Needs work