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.

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

Painless: Decouple PainlessLookupBuilder and Whitelists #32346

Merged
merged 8 commits into from
Jul 25, 2018

Conversation

jdconrad
Copy link
Contributor

Mostly a mechanical change. This moves all code related to the whitelists to a WhitelistsToPainlessLookup class so that PainlessLookupBuilder has no dependencies on the whitelists to build a PainlessLookup. This allows for Painless to create a Lookup from other sources moving forward which may have practical applications in both testing and jar separation shortly.

@jdconrad jdconrad added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.0.0 >refactoring v6.4.0 labels Jul 24, 2018
@jdconrad jdconrad requested a review from rjernst July 24, 2018 20:39
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@jdconrad jdconrad mentioned this pull request Jul 24, 2018
23 tasks
@jdconrad
Copy link
Contributor Author

@rjernst Removed the extraneous class and put whitelistsToPainlessLookup into PainlessLookupBuilder as we discussed.

@jdconrad
Copy link
Contributor Author

@elasticmachine test this please

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, LGTM. Just one minor suggestion.

@@ -126,14 +126,55 @@ public int hashCode() {
private static final Pattern METHOD_NAME_PATTERN = Pattern.compile("^[_a-zA-Z][_a-zA-Z0-9]*$");
private static final Pattern FIELD_NAME_PATTERN = Pattern.compile("^[_a-zA-Z][_a-zA-Z0-9]*$");

private final List<Whitelist> whitelists;
public static PainlessLookup whitelistsToPainlessLookup(List<Whitelist> whitelists) {
Copy link
Member

Choose a reason for hiding this comment

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

I would name this more simply like buildFromWhitelist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jdconrad jdconrad added v6.5.0 and removed v6.4.0 labels Jul 25, 2018
@jdconrad
Copy link
Contributor Author

@rjernst Thanks for the review. Will commit as soon as CI passes.

@jdconrad jdconrad merged commit 853aa0a into elastic:master Jul 25, 2018
jdconrad added a commit that referenced this pull request Jul 25, 2018
Implements a static function in PainlessLookupBuilder that contains all the logic related
to Whitelist.  PainlessLookupBuilder is available for use in loading from methods beyond
Whitelist now.
dnhatn added a commit that referenced this pull request Jul 26, 2018
* master:
  [DOCS] Fix formatting error in Slack action
  Painless: Fix documentation links to use existing refs (#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (#32346)
  [DOCS] Adds recommendation for xpack.security.enabled (#32345)
  [TEST] Mute ConvertProcessortTests.testConvertIntHexError
  [TEST] Fix failure due to exception message in java11 (#32321)
  [DOCS] Fixes typo in ML aggregations page
  [DOCS] Adds link from bucket_span property to common time units
  [ML][DOCS] Add documentation for detector rules and filters (#32013)
  Add opaque_id to index audit logging (#32260)
  Add 6.5.0 version to master
  fixes broken build for third-party-tests (#32353)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jul 26, 2018
* master:
  Only enforce password hashing check if FIPS enabled (elastic#32383)
  [DOCS] Fix formatting error in Slack action
  Painless: Fix documentation links to use existing refs (elastic#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (elastic#32346)
dnhatn added a commit that referenced this pull request Jul 27, 2018
* 6.x:
  Only enforce password hashing check if FIPS enabled (#32383)
  Introduce fips_mode setting and associated checks (#32326)
  [DOCS] Fix formatting error in Slack action
  Ingest: Support integer and long hex values in convert (#32213)
  Release pipelined request in netty server tests (#32368)
  Add opaque_id to index audit logging (#32260)
  Painless: Fix documentation links to use existing refs (#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (#32346)
  [DOCS] Adds recommendation for xpack.security.enabled (#32345)
  [test] package pre-install java check (#32259)
  [DOCS] Adds link from bucket_span property to common time units
  [DOCS] Fixes typo in ML aggregations page
  [ML][DOCS] Add documentation for detector rules and filters (#32013)
  Bump the 6.x branch to 6.5.0 (#32361)
  fixes broken build repository-s3 for third-party-tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants