Skip to content
This repository has been archived by the owner on Jan 27, 2023. It is now read-only.

Copy pipeline dirs to ingest_pipeline #108

Merged
merged 1 commit into from
Jul 1, 2020

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Jul 1, 2020

This is to ensure no directories contain -. In a follow up, the directories with - will be removed.

This is to ensure no directories contain -. In a follow up, the directories with `-` will be removed.
@ruflin ruflin requested a review from ycombinator July 1, 2020 14:55
@ycombinator
Copy link
Contributor

ycombinator commented Jul 1, 2020

Hmm, I wonder if symlinks would work instead of actual copies. Then we don't have to worry about old and new contents getting out of sync for the duration that both exist. We can then do the move later when it's safe to remove to older folders. WDYT?

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #108 opened]

  • Start Time: 2020-07-01T14:54:26.441+0000

  • Duration: 7 min 42 sec

Test stats 🧪

Test Results
Failed 0
Passed 12
Skipped 0
Total 12

@ruflin
Copy link
Member Author

ruflin commented Jul 1, 2020

I don't think symlinks will work as Kibana works with the actual files inside the .tar.gz. As soon as elastic/kibana#70320 goes in, these can be removed. I only don't rename them directly because we have a master and 7.x build and it is tricky to merge both in parallel.

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

LGTM.

$ find packages -iname ingest-pipeline -type d | sort > /tmp/old
$ find packages -iname ingest_pipeline -type d | sort > /tmp/new
$ cat /tmp/old | tr \- \_ | diff /tmp/new - | wc -l
       0

@ruflin ruflin merged commit 7afb8bb into elastic:master Jul 1, 2020
@ruflin ruflin deleted the copy-pipelines branch July 1, 2020 18:10
thomasneirynck pushed a commit to thomasneirynck/package-storage that referenced this pull request Sep 28, 2021
Pipelines are input specific. Currently there is no simple way for the integrations manager to know which pipeline should be reference in an input. This introduces the convention that the pipeline name must be a variable which matches one of the pipeline file names in the package.

On creating the pipeline instance, the integrations manager will change and prefix the name. The above convention makes it possible to convert it in both places. As an example: The package has an ingest pipeline `foo.yml` or `foo.json`, in the input it will reference `foo`. Then the integration manager builds the datasource and calls it `bar-foo`. It will the replace the input variable with `bar-foo`.

This also has consequences on elastic/integrations#3. It means the developer building integrations must ensure that no two pipelines have the same name in one package.

To make sure the name in an input matches a pipeline, later on also a validation step should be added.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants