-
Notifications
You must be signed in to change notification settings - Fork 970
Feature: Dynamic resource notices #2087
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
Conversation
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
function Signed-off-by: Marcos Iglesias <[email protected]>
Signed-off-by: Marcos Iglesias <[email protected]>
37d439a
to
1f60fb8
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
a48d421
to
63d823c
Compare
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.
is this file meant to replace this? https://github.com/amundsen-io/amundsen/blob/main/frontend/amundsen_application/static/js/components/Alert/index.tsx
I remember reviewing a few small changes recently that seem to have been made in the index.tsx file but not here. not sure if this one just hasn't been updated, and if so should the index file be deleted as part of this PR?
Signed-off-by: Marcos Iglesias <[email protected]> chore: tweaking colors
Signed-off-by: Marcos Iglesias <[email protected]>
Signed-off-by: Marcos Iglesias <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
Signed-off-by: Ben Dye <[email protected]>
frontend/amundsen_application/static/js/components/Alert/Alert.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Ben Dye <[email protected]> Update frontend/amundsen_application/static/js/components/Alert/Alert.tsx
Signed-off-by: Ben Dye <[email protected]>
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.
LGTM on the FE side
Signed-off-by: Ben Dye <[email protected]>
Great work and addition! Do we any documentation on how to configure and use this feature? @B-T-D |
I can try to add more documentation at some point. There's a bit of documentation in the form of docstrings in the code currently. Basically, you will need to have some separate service that is generating these notices. You then configure a concrete notice client that subclasses |
Summary of Changes
Tests
Added unit tests for the notice endpoint.
Documentation
Docstrings on public classes.
CheckList
Make sure you have checked all steps below to ensure a timely review.