-
Notifications
You must be signed in to change notification settings - Fork 167
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
[RHOAIENG-12866] entire project context refreshed when individual resources are deleted #3437
Conversation
c5ab200
to
4ff015f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3437 +/- ##
==========================================
- Coverage 85.65% 85.65% -0.01%
==========================================
Files 1347 1347
Lines 30680 30670 -10
Branches 8550 8550
==========================================
- Hits 26280 26271 -9
+ Misses 4400 4399 -1
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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.
Tested and works well, now we don't refresh all project resources and only re-fetch the resources as needed.
/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.
Sorry, I missed some issues, see the comments below. I think after that update everything should be good.
frontend/src/pages/projects/screens/detail/storage/StorageList.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsList.tsx
Outdated
Show resolved
Hide resolved
4ff015f
to
9adb82d
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.
Tested, works well
/lgtm
frontend/src/pages/projects/screens/detail/storage/StorageList.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/projects/screens/detail/data-connections/DataConnectionsList.tsx
Outdated
Show resolved
Hide resolved
…ources are deleted
9adb82d
to
63b8ddd
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DaoDaoNoCode, Gkrumbach07 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
https://issues.redhat.com/browse/RHOAIENG-12866
Description
Updated usages of project details context to not refresh all resources after certain actions like the one mentioned in this ticket take place (e.g. deletion of data connections).
How Has This Been Tested?
Inspected browser network - After deleting data connections, additional API requests for other resources (e.g. fetching of notebooks) no longer occur. This can be done to manually test, or the deletion of cluster storage from the project details page.
Since there was no change in functionality, no additional tests were added for this change.
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main