-
Notifications
You must be signed in to change notification settings - Fork 15
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
NETOBSERV-1313 enable feature gated panels by default #402
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
==========================================
- Coverage 57.80% 57.47% -0.33%
==========================================
Files 167 168 +1
Lines 7871 7944 +73
Branches 970 970
==========================================
+ Hits 4550 4566 +16
- Misses 3042 3099 +57
Partials 279 279
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
{ id: 'top_dns_rcode_bar_total', isSelected: false }, | ||
{ id: 'top_avg_rtt_donut', isSelected: false }, | ||
{ id: 'top_avg_rtt_line', isSelected: false } | ||
{ id: 'top_avg_dns_latency_donut', isSelected: true }, |
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.
should we be more selective and not display them all by default? E.g. show donuts, which are more lightweight, than line/bar charts for the same metric ?
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.
I guess it's more a matter of advertising what's available here more that anything don't you think ?
That selection is kept in cache so this will affect only the first time until the user actually configure the panels (or get new panels not yet assigned from new feature / operator update).
/ok-to-test |
New image: It will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=d420a1e make set-plugin-image |
@jpinsonneau This PR seems to be breaking the consolePlugin. I am hitting the below error as soon as I patch the above image. |
Just tried on OCP 4.13.13 and it works fine 🤔 Which version did you tried ? Your react error is pointing #306
Which is not enough to find where it comes from 😞 |
@jpinsonneau Yeah the image works fine, think I was using older release of operator before which may have led to that error idk. Anyways, the DNS panels are seen on default but when I click on RestoreDefaults it removed those panels which is not the case for PacketDrop which dont go away upon click. |
I think you are still facing some cache issue here: I double checked the code and confirm this is the only place where panels defaults are set. Could you please clear your browser cache before giving another try ? Or just change browser to ensure no cache is there. On my side I will investigate to get rid of all of these cache issues. |
/label qe-approved |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpinsonneau 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 |
Description
Show DNS & RTT panels by default (when feature enabled)
Backport: #403
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.