Updated: Comment #7

Problem/Motivation

There is potentially a lot of possible duplicated code for modules trying to interact with versioncontrol at hook_versioncontrol_code_arrival().

Proposed resolution

Create a new plugin type event_processor, which abstracts most of non-specific code and configuration, so implementing a plugin of that type is easier.

Remaining tasks

  • To use a queue instead of directly executing the plugins.
  • Support per repository configuration for event processor plugins. Maybe on another issue, per backend seems to be good enough to start. Also, the right UI to present this is a little tricky. Suggestions are welcome.

User interface changes

Probably add a configuration UI, but it can be also added later. Pending to decide.

API changes

New repository plugin slot event_processors, which has a list of event_processor plugin type plugins to use during hook_versioncontrol_code_arrival().

Original report by @marvil07

The main idea behind this is to help the interaction with hook_versioncontrol_code_arrival(), and to be able to add any number of plugins to a repository that will be executed on code arrival success.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

#443000: When viewing an issue, display a list of commits that reference that issue # might be postponed on this issue.

It's not clear why this is postponed, or what it's waiting for.

What is blocking moving this forward?

marvil07’s picture

@YesCT: This is decided to be on the next major version of versioncontrol (tagged vc-next, so not yet sure if it wil happen on D7 or D8).

marvil07’s picture

Issue summary: View changes
Status: Postponed » Active
Issue tags: -vc-next

Un-postponing after a quick conversation with sdboyer.

marvil07’s picture

Assigned: Unassigned » marvil07

Mention I'm working on this.

marvil07’s picture

Status: Active » Needs review
FileSize
12.14 KB

A start patch, still needs lots of work, NR for bot.

marvil07’s picture

FileSize
7.44 KB
15.39 KB

It still needs more work, i.e. to handle per plugin configuration.

marvil07’s picture

FileSize
26.83 KB
21.65 KB

Finally, something that is working.

NR for testbot.

What is new on this patch:

  • Actually call processor plugins on hook_versioncontrol_code_arrival().
  • Support multiple plugins by plugin slot.
  • Support plugins acrooss different vcs's.
  • Refactor repository getEventProcessors() to use new plugin related methods.
  • Some changes on VersioncontrolPluginConfigurationInterface.
  • Added a per backend configuration page for event processors.
  • Actually use the right variable name to store event processor per backend configuration settings.

General note: the way we are handling different plugins is not consistent, or at least can be a lot more consistent, but that's kind of unrelated.

Still pending:

  • To use a queue instead of directly executing the plugins.
  • Support per repository configuration for event processor plugins. Maybe on another issue, per backend seems to be good enough to start. Also, the right UI to present this is a little tricky. Suggestions are welcome.
marvil07’s picture

Issue summary: View changes

Updated issue summary.

marvil07’s picture

Issue summary: View changes
marvil07’s picture

FileSize
31.31 KB
8.87 KB

What is new in this patch:

  • Do not use backend on internal structure for default event processor data.
  • Load event processor plugins configuration via its getter.
  • Better serialization for VersioncontrolEvent.
  • Declare and use a new queue to run repository event processors.

I think now it is ready a real review before adding it upstream.

drumm’s picture

This looks good on reading the code.

marvil07’s picture

I made an extra change:

  • Store ids on the queue payload instead of full objects.

I have opened a follow-up #2214173: Per repository configuration for event processor plugins, and after testbot agrees I will be adding this to upstream.

marvil07’s picture

FileSize
3.15 KB
32.41 KB
marvil07’s picture

Assigned: marvil07 » Unassigned
Status: Needs review » Fixed

Added to 7.x-1.x.

marvil07’s picture

One follow-up:

  • 9b45f86 Issue #1796144 follow-up: Catch unknown exceptions on event processor plugin execution.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

  • Commit 736023d on 7.x-1.x, drush-vc-sync-unlock by marvil07:
    Issue #1796144: Added an event processor plugin type for repositories....
  • Commit 9b45f86 on 7.x-1.x, drush-vc-sync-unlock by marvil07:
    Issue #1796144 follow-up: Catch unknown exceptions on event processor...

  • Commit 736023d on 7.x-1.x by marvil07:
    Issue #1796144: Added an event processor plugin type for repositories....
  • Commit 9b45f86 on 7.x-1.x by marvil07:
    Issue #1796144 follow-up: Catch unknown exceptions on event processor...