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

Add exclude/include events filters to audit log #38506

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

bbovenzi
Copy link
Contributor

Add the ability to filter the audit logs by events to include/exclude. Send the config values to the UI to act as the default value.

Also, some fixes to the react-select option in the dataset searchbar

Screenshot 2024-03-26 at 12 16 49 PM

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Mar 26, 2024
@eladkal eladkal added the type:improvement Changelog: Improvements label Mar 26, 2024
@eladkal eladkal added this to the Airflow 2.9.0 milestone Mar 26, 2024
Copy link
Collaborator

@dirrao dirrao left a comment

Choose a reason for hiding this comment

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

It's an interesting feature.

@jedcunningham
Copy link
Member

jedcunningham commented Mar 29, 2024

Found a few issues while giving this a spin.

"No options" - when you first enter into the filter form field, it says there are no options. Even when you start typing say "ru", "running" doesn't come up. Is it possible to hide the suggested options entirely? If you do want suggestions to work, it really needs to be a complete list of events for the DAG (which likely isn't trivial to get with the current rest api).

Screenshot 2024-03-29 at 1 47 43 PM

Both included and excluded events - can we maybe disable the "excluded" events form field when the "included" events is populated? Having both on is a little funky and it feels natural for "included" to take higher precedence to me.

Not really related to this change, but I noticed the count isn't using the included/excluded event filter. See #38625.

@Lee-W Lee-W force-pushed the add-event-filters-to-audit-log-ui branch from e6bed54 to d2fa53f Compare April 2, 2024 07:54
@phanikumv
Copy link
Contributor

Found a few issues while giving this a spin.

"No options" - when you first enter into the filter form field, it says there are no options. Even when you start typing say "ru", "running" doesn't come up. Is it possible to hide the suggested options entirely? If you do want suggestions to work, it really needs to be a complete list of events for the DAG (which likely isn't trivial to get with the current rest api).

Screenshot 2024-03-29 at 1 47 43 PM Both included and excluded events - can we maybe disable the "excluded" events form field when the "included" events is populated? Having both on is a little funky and it feels natural for "included" to take higher precedence to me.

Not really related to this change, but I noticed the count isn't using the included/excluded event filter. See #38625.

Let's resolve these findings in a follow-up PR. Merging this so that we can include in 2.9.0

@phanikumv phanikumv merged commit 32c88a2 into apache:main Apr 2, 2024
39 checks passed
@phanikumv phanikumv deleted the add-event-filters-to-audit-log-ui branch April 2, 2024 08:49
@vatsrahul1001
Copy link
Collaborator

I also agree with @jedcunningham that we should remove 'No options' and only keep exclude option rather than having both

ephraimbuddy pushed a commit that referenced this pull request Apr 2, 2024
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
(cherry picked from commit 32c88a2)
ephraimbuddy pushed a commit that referenced this pull request Apr 2, 2024
Co-authored-by: Wei Lee <weilee.rx@gmail.com>
(cherry picked from commit 32c88a2)
idantepper pushed a commit to idantepper/airflow that referenced this pull request Apr 3, 2024

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
idantepper pushed a commit to idantepper/airflow that referenced this pull request Apr 3, 2024

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
mathiaHT pushed a commit to mathiaHT/airflow that referenced this pull request Apr 4, 2024

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024

Co-authored-by: Wei Lee <weilee.rx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:improvement Changelog: Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants