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

Usage of FastURLFilter via ES JSONURLFilterWrapper #1013

Closed
jnioche opened this issue Dec 13, 2022 Discussed in #1012 · 3 comments
Closed

Usage of FastURLFilter via ES JSONURLFilterWrapper #1013

jnioche opened this issue Dec 13, 2022 Discussed in #1012 · 3 comments

Comments

@jnioche
Copy link
Contributor

jnioche commented Dec 13, 2022

Discussed in #1012

Originally posted by sebastian-nagel December 12, 2022
The FastURLFilter configuration file contains a JSON array, not a JSON object. How can it be put into ES?

When I try to put a FastURLFilter configuration file into ES (as described in JSONURLFilterWrapper, ES responds with an error (indicating that ES expects a JSON object):

curl -XPUT 'localhost:9200/config/config/fast.urlfilter.json?pretty' -H 'Content-Type: application/json' -d @fast.urlfilter.json
{
  "error" : {
    "root_cause" : [
      {
        "type" : "mapper_parsing_exception",
        "reason" : "failed to parse"
      }
    ],
    "type" : "mapper_parsing_exception",
    "reason" : "failed to parse",
    "caused_by" : {
      "type" : "not_x_content_exception",
      "reason" : "Compressor detection can only be called on some xcontent bytes or compressed xcontent bytes"
    }
  },
  "status" : 400
}
```</div>
@jnioche
Copy link
Contributor Author

jnioche commented Dec 13, 2022

@sebastian-nagel we could change the format so that it optionally consumes a simple object with a single field, the value of which is the array.
When using with the ES wrapper, you'd need to have the field but if used directly you wouldn't necessarily need to.
What do you think?

@jnioche jnioche added this to the 2.7 milestone Dec 13, 2022
@sebastian-nagel
Copy link
Contributor

so that it optionally consumes a simple object with a single field, the value of which is the array

Yes, makes sense. Ev., could also require a predefined field "config" or "content" or similar. This special case needs to be documented anyway, so it's maybe clearer to require a specific field name.

@jnioche jnioche added enhancement and removed bug labels Dec 13, 2022
@jnioche
Copy link
Contributor Author

jnioche commented Dec 13, 2022

Fixed by allowing an optional top level field. Thanks @sebastian-nagel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants