Prevent API requests from firing when no measures #5446
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lovincyrus
force-pushed
the
cyrus/prevent-requests-no-measures
branch
2 times, most recently
from
dd55743
to
c068e1b
Compare
nishantmonu51
added
the
blocker
lovincyrus
force-pushed
the
cyrus/prevent-requests-no-measures
branch
from
cbf6c2c
to
95d67c3
Compare
|
Recording: CleanShot.2024-09-05.at.11.55.53.mp4 |
|
Latest:
CleanShot.2024-09-05.at.16.58.30.mp4 |
|
Addressed the feedback from our offline sync @ericpgreen2 |
ericpgreen2
requested changes
Sep 8, 2024
web-common/src/features/entity-management/resource-selectors.ts
Outdated
Show resolved
Hide resolved
ericpgreen2
approved these changes
Sep 9, 2024
There was a problem hiding this comment.
This is good for now (pending my last 2 PR comments), though I'll note that this doesn't handle the case for Rill Cloud. We can skip Rill Cloud handling for now because:
- The error case is more likely in Rill Developer (when you're developing your security policy) than in Rill Cloud
- It'll be cleaner to add this logic to Rill Cloud once we move the dashboard's instantiation logic to a SvelteKit
loadfunction (which @AdityaHegde is considering in his "human-readable" URL state work), so IMO we can wait until then.
ericpgreen2
pushed a commit
that referenced
this pull request
Sep 12, 2024
* prevent aggregation and timeseries requests from running when no measures * prevent aggregation and timeseries requests from running when no measures * gracefully fail with no measures * reset * comments * lint * wip * wip * skip firing if no measures * handle no measures before firing * clean up * todos * lint * error banner when parser error, no mock user or mock user is admin * handle more structure for the writable * update error parsing dashboard to banner rather than errorpage * test error banner * clean up * hide banner when error page is on * mock user admin check for error banner * error page for admin/non-admin, no measures available * use the actual measures * hide previous banner error * enable conditionally in useProjectParser * lint * pr feedback * pr feedback * lint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR prevents the API requests from running when there are no measures mentioned in the security policy. Closes #5224
Related: #5432
Set up this project to test this PR: #5224 (comment)