-
Notifications
You must be signed in to change notification settings - Fork 74
feat: account level default ingestion #2839
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
base: main
Are you sure you want to change the base?
Conversation
josevalim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Just some minor nitpicks on the UI bits :)
| prompt: [key: "Choose a source"] | ||
| ) %> | ||
| <small id="source_id" class="form-text text-muted">This source will use this backend as its default ingest destination. Only sources with default ingest backend support enabled are shown.</small> | ||
| <div :if={length(@available_sources || []) > 0}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <div :if={length(@available_sources || []) > 0}> | |
| <div :if={@available_sources != []}> |
| </.button> | ||
| </li> | ||
| </ul> | ||
| <p :if={Enum.empty?(@backend.sources || [])}>No sources configured yet</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this is a pattern from before this pull request, but it would be great to remove the conditionals such as || [], by making sure they are always initialized to a list.
| <i class="fas fa-info-circle"></i> | ||
| <strong>Ingest from all sources:</strong> This backend is configured to receive logs from ALL your sources automatically. | ||
| To limit this backend to specific sources, disable the "Ingest from all sources" option in the backend settings. | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if you folks decide the UI but I wonder if you should move the toggle here, so someone who wants to manage sources does not need to go to another page, then come back here, to control them.
Closes ANL-1138
if backends.default_ingest=true → ingest all sources to default backend ( BigQuery) and all backends with this true flag
if backends.default_ingest=false + source_backends record → ingest only for that Source to default backend (BigQuery) and that specified backend
Added a pill indicator for default ingestion.
Also removed the source backend list from sources page, as that is no longer relevant.
this greatly simplifies the ingestion model.
the old source column is left in as per non-destructive migration policy. will be removed when we eventually do a v2.