-
Notifications
You must be signed in to change notification settings - Fork 49
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
Remove fetch caching #280
Remove fetch caching #280
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #280 +/- ##
==========================================
+ Coverage 12.15% 12.19% +0.03%
==========================================
Files 411 411
Lines 30575 30458 -117
Branches 776 777 +1
==========================================
- Hits 3717 3713 -4
+ Misses 26525 26412 -113
Partials 333 333
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
7f27e64
to
f459f72
Compare
Rebased on |
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
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.
Thanks @kmcginnes !
Minor note, let's also remove the connection cache section from the README:
https://github.com/aws/graph-explorer?tab=readme-ov-file#connection-cache
@michaelnchin Good catch! I've removed that section. Should be good now. |
Issue #, if available: N/A
Description of changes
Context
When the change was made to use POST for database queries (PR #206), it completely broke the local fetch caching that existed before. This makes the caching logic and configuration irrelevant and essentially dead code.
If we desire client side caching for network calls, we should lean on React Query caching, which is already being used by Graph Explorer in some ways. Removing the custom client side caching makes this transition simpler when it comes time.
Disclaimer
Important
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.