Skip to content

Conversation

pquentin
Copy link
Member

@pquentin pquentin commented Sep 4, 2024

This was done purely looking at the YAML test errors. The Put pipeline API has one error left: the Redact processor is still missing.

Note that even when we hit 100%, the work on ingest pipelines won't be over. Not only there are number of pipelines still missing (tracked in #2231), but we have not added all ingest modules to YAML tests.

Copy link
Contributor

github-actions bot commented Sep 4, 2024

Following you can find the validation results for the APIs you have changed.

API Status Request Response
ingest.delete_geoip_database 🟢 1/1 1/1
ingest.delete_pipeline 🟢 15/15 15/15
ingest.geo_ip_stats 🟢 1/1 1/1
ingest.get_geoip_database 🟢 4/4 4/4
ingest.get_pipeline 🟢 22/22 22/22
ingest.processor_grok 🟢 1/1 1/1
ingest.put_geoip_database 🟢 3/3 3/3
ingest.put_pipeline 🔴 58/59 59/59
ingest.simulate 🔴 6/9 9/9

You can validate these APIs yourself by using the make validate target.

@l-trotta
Copy link
Contributor

l-trotta commented Sep 4, 2024

In UserAgentProcessor we should remove options, it's not a field it seems. server code. Other than that, LGTM!

Copy link
Contributor

github-actions bot commented Sep 4, 2024

Following you can find the validation results for the APIs you have changed.

API Status Request Response
ingest.delete_geoip_database 🟢 1/1 1/1
ingest.delete_pipeline 🟢 15/15 15/15
ingest.geo_ip_stats 🟢 1/1 1/1
ingest.get_geoip_database 🟢 4/4 4/4
ingest.get_pipeline 🟢 22/22 22/22
ingest.processor_grok 🟢 1/1 1/1
ingest.put_geoip_database 🟢 3/3 3/3
ingest.put_pipeline 🔴 58/59 59/59
ingest.simulate 🔴 6/9 9/9

You can validate these APIs yourself by using the make validate target.

@pquentin
Copy link
Member Author

pquentin commented Sep 4, 2024

In UserAgentProcessor we should remove options, it's not a field it seems. server code. Other than that, LGTM!

Thanks Laura! Removing options made me realize that we had an UserAgentProperty enum, so I've repurposed it for properties. I also opened elastic/elasticsearch#112518 to fix the relevant Elasticsearch docs.

Please take another look.

@pquentin pquentin mentioned this pull request Sep 4, 2024
Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

LGTM!

@pquentin pquentin merged commit 95c66a9 into main Sep 5, 2024
6 checks passed
@pquentin pquentin deleted the ingest-pipeline-errors branch September 5, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants