elastic / elasticsearch-net

This strongly-typed, client library enables working with Elasticsearch. It is the official client maintained and supported by Elastic.
https://www.elastic.co/guide/en/elasticsearch/client/net-api/current/index.html
Apache License 2.0
14 stars 1.15k forks source link

Fluent configuration of `Remove` processor on `PutPipelineRequestDescriptor` doesn't support field descriptor #8358

Open stevejgordon opened 2 months ago

stevejgordon commented 2 months ago

Is your feature request related to a problem? Please describe. When creating an ingest pipeline in code, some processors support using the TDocument when configuring the Field to act on. Remove does not.

var putPipelineResponse = await es.Ingest.PutPipelineAsync<EncryptedAuthenticationSession>(pipelineName, p => p
    .Description("Adds geo IP and user agent information.")
    .Processors(pr => pr
        .Geoip(g => g.Field(f => f.RemoteIpAddress!).TargetField("geo_ip").IgnoreMissing())
        .UserAgent(u => u.Field(f => f.UserAgentString!).TargetField("user_agent").IgnoreMissing())
        .Remove(r => r.Field("user_agent_string"!))), ct);

See above, we can only provide a string, implicitly converted to Fields.

Describe the solution you'd like Potentially, there's something amiss with the code generation, or we must manually add this to the partial class. It would be helpful if this processor acted like the others.

Describe alternatives you've considered N/A

Additional context Another slight developer experience issue I encountered is that when using the string to define the field, you get a nullability warning. This is due to the implicit conversion potentially returning null.

For example: Image

I use the null forgiving operator in the preceding code snippet to avoid the warning. It would be nice not to have to do that. This might be achieved with additional nullability annotations on the implicit conversion operator.

flobernd commented 2 months ago

The Remove processor uses Fields instead of Field . C# does not do "transient" implicit conversions here, which is a little bit inconvenient .. Fields supports conversion from e.g. string[], but not for string. I think I could add implicit conversions for the scalar types as well to make it a bit easier to use.

I use the null forgiving operator in the preceding code snippet to avoid the warning. It would be nice not to have to do that. This might be achieved with additional nullability annotations on the implicit conversion operator.

I planned to use the NotNullIfNotNull annotation, but I have to do some internal refactoring first, because currently the operators for Fields might as well return null, if the input is an empty array or an array that contains only string.Empty values etc.