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

Jump to bottom

INGEST: Make a few Processors callable by Painless #32170

Merged
merged 7 commits into from
Jul 20, 2018
Merged

INGEST: Make a few Processors callable by Painless #32170

merged 7 commits into from
Jul 20, 2018

Conversation

original-brownbear
Copy link
Member

  • Extracted a few stateless String processors as well as the json processor to static methods and whitelisted them in Painless

@original-brownbear
INGEST: Make a few Processors callable by Painless
43047f8 
* Extracted a few stateless String processors as well as the json processor to static methods and whitelisted them in Painless
@original-brownbear original-brownbear added >enhancement WIP :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP labels Jul 18, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@original-brownbear
Copy link
Member Author

@rjernst just looking for a quick confirmation that this (putting all the stateless things behind on a single Processors class and just acting on field values mostly) is on the right track with you here :)

@rjernst
Copy link
Member

rjernst commented Jul 18, 2018 •
edited
Loading

Thanks @original-brownbear. This is generally what I was thinking. The only caveat is I would try to keep the implementations in the processors themselves as static methods, and have the static methods on Processors call the static methods on the implementation classes. In the future, when we have static imports, Processors can then just go away, static importing from each class.

@original-brownbear
Copy link
Member Author

@rjernst yea

The only caveat is I would try to keep the implementations in the processors themselves as static methods, and have the static methods on Processors call the static methods on the implementation classes.

I'd like the same but can't really do that I'm afraid :( The processors all live in ingest-common but I can't whitelist methods from there because these classes aren't on the CP when the whitelisting happens. Is there a way to whitelist stuff from ingest-common that I couldn't find?

@rjernst
Copy link
Member

rjernst commented Jul 19, 2018

Yes, it is possible. See the painless whitelist example plugin.

@original-brownbear
Copy link
Member Author

@rjernst alright done I think. Let me know if this is the right approach. There's actually not that many processors left that make sense to expose here (the others either have some state and would be inefficient to expose for now or they're just simple String operations that we've whitelisted already like String#trim) and I'd add those if you're generally ok with this thing now :)

@original-brownbear
Copy link
Member Author

original-brownbear commented Jul 20, 2018 •
edited
Loading

@rjernst Can you take another look here when you have a sec? Merging this one would also allow opening a clean PR for my drop solution I wrote up yesterday (that kinda needs the whole whitelisting magic added here for the drop() method :)).

rjernst
rjernst approved these changes Jul 20, 2018
View reviewed changes
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. LGTM.

@original-brownbear
Copy link
Member Author

@rjernst np + thanks for the review => drop call incoming :). Also, just 7.0 here right?

@original-brownbear original-brownbear merged commit e21692e into elastic:master Jul 20, 2018
@original-brownbear original-brownbear deleted the invoke-processors-in-painless branch July 20, 2018 19:10
@rjernst
Copy link
Member

rjernst commented Jul 20, 2018

I think this can go to 6.x.

original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Jul 21, 2018
@original-brownbear
INGEST: Make a few Processors callable by Painless (elastic#32170)
8ab82a6 
* INGEST: Make a few Processors callable by Painless
* Extracted a few stateless String processors as well as the json processor to static methods and whitelisted them in Painless
* provide whitelist from processors plugin
@original-brownbear original-brownbear mentioned this pull request Jul 21, 2018
Merged
martijnvg added a commit that referenced this pull request Jul 21, 2018
@martijnvg
Merge remote-tracking branch 'es/master' into ccr
8ed3624 
* es/master: (23 commits)
  Switch full-cluster-restart to new style Requests (#32140)
  [DOCS] Clarified that you must remove X-Pack plugin when upgrading from pre-6.3. (#32016)
  Remove BouncyCastle dependency from runtime (#32193)
  INGEST: Extend KV Processor (#31789) (#32232)
  INGEST: Make a few Processors callable by Painless (#32170)
  Add region ISO code to GeoIP Ingest plugin (#31669)
  [Tests] Remove QueryStringQueryBuilderTests#toQuery class assertions (#32236)
  Make sure that field aliases count towards the total fields limit. (#32222)
  Switch rolling restart to new style Requests (#32147)
  muting failing test for internal auto date histogram to avoid failure before fix is merged
  MINOR: Remove unused `IndexDynamicSettings` (#32237)
  Fix multi level nested sort (#32204)
  Enhance Parent circuit breaker error message (#32056)
  [ML] Use default request durability for .ml-state index (#32233)
  Remove indices stats timeout from monitoring docs
  Rename ranking evaluation response section (#32166)
  Dependencies: Upgrade to joda time 2.10 (#32160)
  Remove aliases resolution limitations when security is enabled (#31952)
  Ensure that field aliases cannot be used in multi-fields. (#32219)
  TESTS: Check for Netty resource leaks (#31861)
  ...
original-brownbear added a commit that referenced this pull request Jul 21, 2018
@original-brownbear
INGEST: Make a few Processors callable by Painless (#32170) (#32261)
113d6d5 
* INGEST: Make a few Processors callable by Painless
* Extracted a few stateless String processors as well as the json processor to static methods and whitelisted them in Painless
* provide whitelist from processors plugin
dnhatn added a commit that referenced this pull request Jul 22, 2018
@dnhatn
Merge branch '6.x' into ccr-6.x
566f9b4 
* 6.x:
  Improve message when JAVA_HOME not set (#32022)
  INGEST: Make a few Processors callable by Painless (#32170) (#32261)
  INGEST: Extend KV Processor (#31789) (#32232) (#32262)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjernst rjernst rjernst approved these changes

Assignees
No one assigned
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement v6.4.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@original-brownbear @elasticmachine @rjernst @colings86