Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teach pipeline validator about terminate processor #847

Closed
wants to merge 2 commits into from
Closed

Teach pipeline validator about terminate processor #847

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb47636
Teach pipeline validator abount terminate processor
efd6 Dec 16, 2024
d304004
address pr comment
efd6 Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions spec/changelog.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
- description: Add support for semantic_text field definition.
type: enhancement
link: https://github.com/elastic/package-spec/pull/807
- description: Add `terminate` processor to list pipeline spec.
type: enhancement
link: https://github.com/elastic/package-spec/pull/847
Comment on lines +13 to +15
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be added in a 3.3.2. Both spec 3.3 and the terminate processor were introduced in 8.16,

Suggested change
- description: Add `terminate` processor to list pipeline spec.
type: enhancement
link: https://github.com/elastic/package-spec/pull/847
- version: 3.3.2-next
changes:
- description: Add `terminate` processor to list pipeline spec.
type: enhancement
link: https://github.com/elastic/package-spec/pull/847

- version: 3.3.1
changes:
- description: Add validation rule to ensure security capability is added if there is any security rule asset.
Expand Down
5 changes: 5 additions & 0 deletions spec/integration/elasticsearch/pipeline.spec.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ spec:
# set_security_user: false # Applicable to packages?
sort: { type: object }
split: { type: object }
terminate: { type: object }
Copy link
Member

Choose a reason for hiding this comment

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

Btw, please add a patch below, so this processor is actually not available in older versions of the spec, something like this:

  - before: 3.3.0
    patch:
      - op: remove
        path: /definitions/processors/terminate # remove terminate processor validation

trim: { type: object }
uppercase: { type: object }
urldecode: { type: object }
Expand Down Expand Up @@ -115,3 +116,7 @@ versions:
path: /definitions/processors/if # remove rename processor validation
- op: remove
path: /definitions/processors/then # remove rename processor validation
- before: 3.3.0
patch:
- op: remove
path: /definitions/processors/terminate # remove terminate processor validation
Comment on lines +119 to +122
Copy link
Contributor

Choose a reason for hiding this comment

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

This array item should be added as the first one (as it references a newer version in the before field).

And looking at the error:

failed to load schema for "integration/elasticsearch/pipeline.spec.yml": failed to apply patch: error in remove for path: '/definitions/processors/terminate': unable to remove nonexistent key: terminate: missing value

the path should be updated too.

I think it should be like this:

versions:
  - before: 3.3.0
    patch:
      - op: remove
        path: /definitions/processor/properties/terminate # remove terminate processor validation
  - before: 3.1.0
    patch:
      - op: remove
        path: /definitions/processors/if # remove rename processor validation
      - op: remove
        path: /definitions/processors/then # remove rename processor validation