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

ingest/filtering: enable filtering by default #4906

Closed
sreuland opened this issue Jun 14, 2023 · 1 comment · Fixed by #4955
Closed

ingest/filtering: enable filtering by default #4906

sreuland opened this issue Jun 14, 2023 · 1 comment · Fixed by #4955

Comments

@sreuland
Copy link
Contributor

sreuland commented Jun 14, 2023

What problem does your feature solve?

un-necessary configuration flag to enable ingestion filtering rather than default

What would you like to see?

ingestion filtering enabled by default, don't need to configure the runtime flag, remove --exp-enable-ingestion-filtering flag from the help output, but keep the flag definition. Log a warning message saying the flag is deprecated if it's present. find any references to the flag in the repo and remove them or document as deprecated.

There will be no filter rules created by default which results in effectively same end result, nothing filtered. Need to confirm this is the outcome when filtering enabled with no rules.

remove reference of the flag in the public ingestion docs

What alternatives are there?

set the --exp-enable-ingestion-filtering in horizon runtime config.

@mollykarcher mollykarcher changed the title /ingest/filtering: enable filtering by default ingest/filtering: enable filtering by default Jun 14, 2023
@mollykarcher
Copy link
Contributor

Let's check the expected behavior when an unknown flag is sent; will installs fail or silently ignore it. This will impact how we want to roll it out - with deprecation warning or just hard remove

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

Successfully merging a pull request may close this issue.

3 participants