Skip to content
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-1266 Netflow traffic tab crash #377

Merged
merged 5 commits into from
Sep 4, 2023

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented Aug 29, 2023

NetflowTab props is updating a log and retrigger NetflowTraffic page endlessly.

  • added a state + check on previous prop values to avoid infinite loop
  • ensure navFunc is loaded when navigate is called

@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch coverage: 53.06% and project coverage change: -0.28% ⚠️

Comparison is base (4a19874) 57.67% compared to head (5db0b6e) 57.39%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #377      +/-   ##
==========================================
- Coverage   57.67%   57.39%   -0.28%     
==========================================
  Files         166      167       +1     
  Lines        7712     7798      +86     
  Branches      935      938       +3     
==========================================
+ Hits         4448     4476      +28     
- Misses       2993     3054      +61     
+ Partials      271      268       -3     
Flag Coverage Δ
uitests 58.33% <50.00%> (+0.34%) ⬆️
unittests 54.82% <66.66%> (-1.96%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
cmd/plugin-backend.go 0.00% <0.00%> (ø)
pkg/server/server.go 70.00% <ø> (ø)
web/src/components/netflow-traffic.tsx 56.13% <ø> (+0.36%) ⬆️
web/src/model/config.ts 100.00% <ø> (ø)
web/src/components/netflow-tab.tsx 57.14% <42.30%> (+7.14%) ⬆️
...b/src/components/dynamic-loader/dynamic-loader.tsx 66.66% <58.33%> (+6.66%) ⬆️
pkg/handler/frontend-config.go 39.39% <83.33%> (+3.91%) ⬆️
pkg/server/routes.go 89.65% <100.00%> (ø)
web/src/components/query-summary/summary-panel.tsx 46.51% <100.00%> (+0.84%) ⬆️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Amoghrd
Copy link
Contributor

Amoghrd commented Aug 31, 2023

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 31, 2023
@Amoghrd
Copy link
Contributor

Amoghrd commented Aug 31, 2023

oh didnt see the branch is out of date with master before putting ok-to-test label. If you could rebase with master @jpinsonneau I can go ahead with testing it if its ready

@github-actions
Copy link

New image:
quay.io/netobserv/network-observability-console-plugin:f12ccc4

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=f12ccc4 make set-plugin-image

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 31, 2023
@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Aug 31, 2023
@jpinsonneau
Copy link
Contributor Author

oh didnt see the branch is out of date with master before putting ok-to-test label. If you could rebase with master @jpinsonneau I can go ahead with testing it if its ready

Done; it seems the testing is failling so I'll take a look tomorrow about that.
Thanks !

@github-actions
Copy link

New image:
quay.io/netobserv/network-observability-console-plugin:288c675

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=288c675 make set-plugin-image

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 1, 2023
@jpinsonneau
Copy link
Contributor Author

Fixed testing since enzyme doesn't manage useEffect hook 5d65897

Also took the opportunity to add some tab examples to standalone mode 2bbe320
image
image

@jpinsonneau jpinsonneau added the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 1, 2023
@jpinsonneau
Copy link
Contributor Author

I'm generating a new image just in case but there are no changes in the behavior if you already started your testing @Amoghrd 👍

switch (id) {
case 'netflow-traffic-parent':
case 'pod-tab':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the rationale for having here only pod/namespace/node and not all the supported kinds? Is it only used in tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it's just for local dev purposes and it's hardcoding "test" as name and "default" as namespace

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

New image:
quay.io/netobserv/network-observability-console-plugin:f85374c

It will expire after two weeks.

To deploy this build, run from the operator repo, assuming the operator is running:

USER=netobserv VERSION=f85374c make set-plugin-image

Copy link
Member

@jotak jotak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code LGTM (not tested)

@openshift-ci openshift-ci bot added the lgtm label Sep 1, 2023
@Amoghrd
Copy link
Contributor

Amoghrd commented Sep 1, 2023

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved QE has approved this pull request label Sep 1, 2023
@openshift-ci openshift-ci bot removed the lgtm label Sep 4, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 4, 2023

New changes are detected. LGTM label has been removed.

@github-actions github-actions bot removed the ok-to-test To set manually when a PR is safe to test. Triggers image build on PR. label Sep 4, 2023
@openshift-ci
Copy link

openshift-ci bot commented Sep 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 1ca3b6a into netobserv:main Sep 4, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm qe-approved QE has approved this pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants