Problem/Motivation

At the moment, indexing to elastic search is hard-coded.
Feasibly you may want to store the data in json, or somewhere else.

Proposed resolution

Add a config entity for 'output'
Add a plugin system for 'output'
Config entity takes a configured output plugin.
E.g. there would be an Elasticsearch destination that has configuration for the index URL and auth etc.

Remaining tasks

All of the above

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 Screenshot from 2021-11-30 11-37-55.png98.57 KBkim.pepper
#4 Screenshot from 2021-11-30 11-35-38.png135.52 KBkim.pepper

Issue fork data_pipelines-3236171

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

larowlan created an issue. See original summary.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Assigned: Unassigned » kim.pepper

Taking a look

kim.pepper opened merge request !4

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Active » Needs review
StatusFileSize
new Screenshot from 2021-11-30 11-35-38.png135.52 KB
new Screenshot from 2021-11-30 11-37-55.png98.57 KB

Here's a couple of screenshots of work in progress.

Screenshot of dataset destinations

Next steps:

  1. Rename 'index' to 'process' since we're doing more than indexing datasets now
  2. Add a button 'Save and process' as well as just 'Save' so we don't have to always process just to add/update a dataset
  3. Finish implementing the elasticsearch plugin. I've split the plugin into its own sub-module data_pipelines_elasticsearch so we don't have a hard dependency on elasticsearch
  4. Update tests.

Reviews of the work in progress are welcome!

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

This is ready for review now. Thanks!

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

The module is alpha state so an upgrade path is not required but the module is being used on prod so let's add an upgrade path for config schema changes and enabling the new module.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

How would this help with #3229333: Add API to fetch data using destination plugins?

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Assigned: kim.pepper » Unassigned

Not actively working on this anymore.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Here goes my whole Saturday. :D

  • Merged 1.x branch along with merge conflicts.
  • Fixed update hooks and post-update hook logic.
  • User now have to create Elasticserch destination right after data_pipelines_update_8008. We should add this to release notes.
  • Multiple destinations are working great now.
  • Fixed failing tests.
  • This will make #3259685: Allow skipping indexing on Dataset save obsolete.
  • Left a couple of questions.
  • Other than that this is ready to test.
  • I'll try to create a PR for our project.
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

I think we are good to go after 7 update hooks. The internal PR is up and running as well.

mortim07 made their first commit to this issue’s fork.

mortim07’s picture

mortim07 commented
Status: Needs review » Reviewed & tested by the community

  • jibran committed 6a5c2fb on 1.x 
    Issue #3236171 by kim.pepper, jibran, mortim07: Add pluggable outputs
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 1.x.
Thanks all for the mammoth effort.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented

Woo! 🎉

mortim07’s picture

mortim07 commented

We got there! 🎉 Thanks all!

jibran closed merge request !4

mortim07’s picture

mortim07 commented
Status: Fixed » Closed (fixed)
kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Status: Closed (fixed) » Fixed

Lets keep this at fixed for any feedback until it auto-closes in a couple of weeks.

mortim07’s picture

mortim07 commented

@kim.pepper Sure thing.

Status: Fixed » Closed (fixed)

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