Skip to content

feat: Add sharable cache #161

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

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

feat: Add sharable cache #161

wants to merge 5 commits into from

Conversation

priya-ni
Copy link
Contributor

@priya-ni priya-ni commented May 2, 2025

Pull Request

🤨 Rationale

To add a new singleton class that will be used as a shared cache for the upcoming Work Orders and Test Plans data sources.

In the work orders and test plans data sources, query builders will be used to add filters. For fields such as Updated by, Requested by, Assigned to, and Created by, dropdowns displaying usernames and email IDs will be provided. To avoid repeatedly querying user details each time a data source is selected, a shared cache will be used to store these details once and make them accessible across all data sources. Similarly this will be used to cache workspaces, Products part numbers, systems.

Please refer to this HLD for more details on how this cache will be used.

👩‍💻 Implementation

  • Created SharedCache class with store map property that used to set & get values by key
  • each value will be stored with time to live value, the default value will be 10 minutes
  • Created a global instance which will be used in needed places

🧪 Testing

Added unit tests

✅ Checklist

Copy link
Collaborator

@atmgrifter00 atmgrifter00 left a comment

Choose a reason for hiding this comment

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

Have questions regarding this change that need to be answered.

@priya-ni priya-ni requested a review from atmgrifter00 May 14, 2025 08:36
@priya-ni
Copy link
Contributor Author

@rajsite @atmgrifter00 @cameronwaterman Could you please review this PR?

@atmgrifter00
Copy link
Collaborator

atmgrifter00 commented May 14, 2025

@priya-ni, I think there are still some very fundamental issues with the approach you are taking here. First, the fact that the SharedCache is generic doesn't really provide any significant difference in its typing, and in fact, is still used effectively with any for its value typing. The cleanup code running on a timer also is concerning, as this seems like it could be the source of unexpected behaviors/bugs.

Ultimately, it would be good to understand if there are patterns/mechanisms that the frameworks we are building on top of (i.e. Grafana/React) that handle concerns such as this. It seems like what we want is a dependency injection system with a service that can then be injected where it is needed (as we do in Angular).

I believe we intend to discuss this PR at tomorrow's Soliton Tech Syncup (05/14). Hopefully after that meeting we can provide some better direction.

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

Successfully merging this pull request may close these issues.

5 participants