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

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

Open
Tracked by #8356
stevejgordon opened this issue Sep 25, 2024 · 1 comment
Labels

Comments

@stevejgordon
Copy link
Contributor

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 flobernd added 8.x Relates to 8.x client version Area: Client labels Sep 25, 2024
@flobernd
Copy link
Member

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.

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

No branches or pull requests

2 participants