LI-104: Add support for lookup processor #415
Conversation
ghost
added
| @@ -97,6 +98,16 @@ resource "datadog_logs_custom_pipeline" "my_pipeline_test" { | |||
| sources = ["ip1"] | |||
| } | |||
| } | |||
| processor { | |||
| lookup_processor { | |||
| name = "lookup processor" | |||
There was a problem hiding this comment.
we should start only with required fields
There was a problem hiding this comment.
What's the reasoning? Because all the processors around in the test are defined that way.
There was a problem hiding this comment.
we should make sure that optional fields are really optional. you are right that there is no example for this case.
There was a problem hiding this comment.
Still this is a good point, we should create such test. This makes us realize that the same kind of issue may be elsewhere.
There was a problem hiding this comment.
I have added a test with only the mandatory fields.
There was a problem hiding this comment.
Reviewed with @nmuesch, @bkabrda and @zippolyte
Regarding the casting on optional fields there seem to be an issue open as well https://github.com/terraform-providers/terraform-provider-datadog/issues/387
Potentially, we will need to fix this on other resources as well (and add tests without optional fields) but let's make sure it just works well for the scope of this PR. Then we should tackle other processors in a distinct PR.
| @@ -97,6 +98,16 @@ resource "datadog_logs_custom_pipeline" "my_pipeline_test" { | |||
| sources = ["ip1"] | |||
| } | |||
| } | |||
| processor { | |||
| lookup_processor { | |||
| name = "lookup processor" | |||
There was a problem hiding this comment.
Still this is a good point, we should create such test. This makes us realize that the same kind of issue may be elsewhere.
| Required: true, | ||
| Elem: &schema.Schema{Type: schema.TypeString}, | ||
| }, | ||
| "default_lookup": {Type: schema.TypeString, Optional: true}, |
There was a problem hiding this comment.
Should this be marked as required? It looks like trying to create a lookup processor without this value doesn't work.
There was a problem hiding this comment.
It is optional on the API side, what is the error that you get when you try to create a lookup processor without default lookup?
There was a problem hiding this comment.
I'm getting this:
Error: error updating logs pipeline: (API error 400 Bad Request: {"error":{"code":"InvalidArgument","message":"Invalid Pipeline","details":[{"code":"InvalidArgument","message":"Invalid default lookup: must not be blank"}]}})
@blemale could you please take a look?
blemale
force-pushed
the
bastien.lemale/LI-104_Add_support_for_lookup_processor
branch
4 times, most recently
from
a89581d to
d203877
Compare
blemale
force-pushed
the
bastien.lemale/LI-104_Add_support_for_lookup_processor
branch
4 times, most recently
from
cbc80de to
e0e8358
Compare
The format of the lookup_table field is `["key1,value1", "key2,value2"]` and not `["key1", "value1", "key2", "value2"]`
blemale
force-pushed
the
bastien.lemale/LI-104_Add_support_for_lookup_processor
branch
from
e0e8358 to
c2954d4
Compare
Add a test with only mandatory fields for lookup processor and make default_lookup effectively optional.
blemale
force-pushed
the
bastien.lemale/LI-104_Add_support_for_lookup_processor
branch
from
c2954d4 to
78657a3
Compare
Support: