ui: show warning when data is not old enough#109164
ui: show warning when data is not old enough#109164craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
@florence-crl can you provide feedback on the tooltip text? |
|
How about "Data is available since MMM DD, HH:MM (UTC)"? Have we been using "data" as a generic term or "SQL Stats" is the right term here? Or "Latest data available since MMM DD, HH:MM (UTC)"? I'm also thinking that we should inform users in the Search Criteria section of this information as well so they have it before making a selection. And disabling the dropdown selections that we don't have data for. |
gtr
left a comment
There was a problem hiding this comment.
I'm also thinking that we should inform users in the Search Criteria section of this information as well so they have it before making a selection. And disabling the dropdown selections that we don't have data for.
I think disabling the dropdown would defeat the purpose of this warning, no? But I do agree that informing users in the search criteria about the data we have is a good idea. That way, it comes as less of a surprise when the warning does appear.
Reviewed 14 of 14 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @florence-crl, @koorosh, and @maryliag)
pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/aggregatedLabel.tsx line 33 at r1 (raw file):
} export const TimeScaleLabel: React.FC<TimeScaleLabelProps> = ({
small nit: the file is called aggregatedLabel.tsx but the exported component here is TimeScaleLabel. I think naming it something like AggregatedTimeScaleLabel is better since it'll match the filename and it'll be used in the aggregated stats pages. I could also see it just being called TimeScaleLabel since it's also used in the txn details and stmt details pages, though. Either way, up to you but I think the filename should match the component name.
If a user selects a time period that we don't have the data for it (because it was already cleaned), we show a warning on SQL Activity (Statement and Transactions) to indicate the data might not be complete. Fix cockroachdb#108989 Release note (ui change): Show warning when time period selected on SQL Activity is older than the oldest data available.
maryliag
left a comment
There was a problem hiding this comment.
Updates to "SQL Stats available are available since". Image updated on PR description.
There is another issue to focus on the Search Criteria, so I will leave those changes to it.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/aggregatedLabel.tsx line 33 at r1 (raw file):
Previously, gtr (Gerardo Torres Castro) wrote…
small nit: the file is called
aggregatedLabel.tsxbut the exported component here isTimeScaleLabel. I think naming it something likeAggregatedTimeScaleLabelis better since it'll match the filename and it'll be used in the aggregated stats pages. I could also see it just being calledTimeScaleLabelsince it's also used in the txn details and stmt details pages, though. Either way, up to you but I think the filename should match the component name.
Done
gtr
left a comment
There was a problem hiding this comment.
Reviewed 5 of 6 files at r2, all commit messages.
Reviewable status:
|
Talked offline with Dogni and the copy was approved. bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 0d123cf to blathers/backport-release-23.1-109164: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
If a user selects a time period that we don't have the data for it (because it was already cleaned),
we show a warning to indicate the data might not be complete.
Fix #108989
https://www.loom.com/share/a7e0b0738d834bb0b6f38a9da43fb6a4
Release note (ui change): Show warning when time period selected on SQL Activity is older than the oldest data available.