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

FEAT: Unified Notifications Feature Implementation #1089

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

tahierhussain
Copy link
Contributor

What

Implemented the Unified Notifications feature, which includes:

  1. Storing log messages in Redis for 24 hours to persist.
  2. Storing popup notification messages in Redis alongside log messages.
  3. Enhancing the logs component to display both log and notification messages.
  4. Making the logs component global, now available on all pages.
  5. Adding UI/UX improvements, allowing users to resize the logs component for better usability.

Why

  1. Previously, log messages were not stored, causing them to be lost on page refresh.
  2. Popup notifications were short lived, making it difficult for users to track them after they disappeared.
  3. The logs component was only available on specific pages, leading to inconsistent behavior and limited utility across the application.
  4. The existing logs component lacked flexibility and usability, making it less user-friendly.

How

  1. Integrated Redis to persist log and notification messages for 24 hours.
  2. Updated the logs component to fetch and display both log and notification messages.
  3. Refactored the logs component to make it global, ensuring it appears at the bottom of every page.
  4. Enhanced the UI/UX of the logs component by adding resizable functionality, allowing users to adjust its height as needed.
  5. Improved the overall design and layout of the logs component for better user experience.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

Yes. This PR includes some major refactoring with respect to the way logs and popup notifications are handled.

Database Migrations

NA

Env Config

backend's .env:
LOGS_EXPIRATION_TIME_IN_SECOND: This env stores the expiration time of the log messages.

Relevant Docs

NA

Related Issues or PRs

NA

Dependencies Versions

NA

Notes on Testing

NA

Screenshots

Logs component collapsed:

Screenshot from 2025-01-24 09-18-28

Logs component semi-expanded:

Screenshot from 2025-01-24 09-18-14

Logs component fully expanded:

Screenshot from 2025-01-24 09-18-21

Checklist

I have read and understood the Contribution Guidelines.

Copy link
Contributor

filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_logs}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_client\_init}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_run\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#23d18b}{\tt{9}}$$

Copy link

return Response({"data": sorted_logs}, status=status.HTTP_200_OK)

@action(detail=False, methods=["post"])
def store_log(self, request: HttpRequest) -> Response:
Copy link
Contributor

Choose a reason for hiding this comment

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

@tahierhussain if we are already storing logs in redis before publishing the message - why do we need this API?

@chandrasekharan-zipstack
Copy link
Contributor

@tahierhussain how will this behave across prompt studio and workflow screens? I believe we show different log headers for these 2 pages currently. Can you add a screenshot for the workflow / pipeline pages as well in that regard?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants