Show FG_NHG CLI Commands Added #1056

kktheballer · 2020-08-18T00:13:18Z

show fg-nhg-hash-view
(shows the current hash bucket view of fg nhg)
show fg-nhg-active-hops
(shows which set of next-hops are active)

- What I did

- How I did it

- How to verify it

- Previous command output (if the output of a command-line utility has changed)

- New command output (if the output of a command-line utility has changed)

lgtm-com · 2020-08-18T00:19:31Z

This pull request introduces 4 alerts when merging 1f4e1a84c842e715ce72b16534325e9422b842fe into 8768580 - view on LGTM.com

new alerts:

4 for Except block handles 'BaseException'

lgtm-com · 2020-08-18T01:52:35Z

This pull request introduces 6 alerts when merging 5071da84c178eda8f6133869c363d8cc7445dc2f into 8768580 - view on LGTM.com

new alerts:

6 for Except block handles 'BaseException'

anish-n · 2020-08-18T01:55:23Z

show/main.py

+	elif ":" in tk:
+		key_avail['ipv6'] = tk
+
+    if nhg == "fgnhg_v4":


Cannot hardcode name checks here, the user may configure any name they desire

anish-n · 2020-08-18T01:55:35Z

show/main.py

+		click.echo("IPV4 nhg does not exist!")
+		exit()
+
+    if nhg == "fgnhg_v6":


Cannot hardcode name checks here, the user may configure any name they desire. Same comment throughout the PR as well.

lgtm-com · 2020-08-18T02:14:12Z

This pull request introduces 6 alerts when merging b2954398a5ca8e7ba10aec7a59e0ed36ff35b6bc into 8768580 - view on LGTM.com

new alerts:

6 for Except block handles 'BaseException'

lguohan · 2020-08-18T15:34:42Z

please add unit test and also check the coverage report generated.

lguohan

please add unit test and also check the coverage report generated.

lguohan · 2020-08-19T11:29:22Z

please address lgtm warnings

lguohan · 2020-08-19T11:30:17Z

show/main.py

@@ -1917,6 +1917,210 @@ def mmu():
    click.echo(proc.stdout.read())


+#


since this is a new group, please separate into a different file. check show/vlan.py for example.

lguohan · 2020-08-19T11:31:09Z

show/main.py

+@cli.command('fg-nhg-active-hops')
+@click.argument('nhg', required=True)
+def fg_nhg_active_hops(nhg):
+    state_db = SonicV2Connector(host='127.0.0.1')


please use pass_db decorator.

For some reason, when locally testing using pass_db decorator I wasn't able to fetch my config_db table using "db.cfgdb.get_table()".

Only when using SonicV2Connector was I able to fetch my config_db table. Any suggestions?

lguohan · 2020-08-19T11:31:24Z

show/main.py

+    try:
+        state_db.connect(state_db.STATE_DB, False)  # Make one attempt only STATE_DB
+    except:
+        click.echo("STATE_DB Table does not exist!")


use ctx.fail()

lgtm-com · 2020-09-08T02:49:43Z

This pull request introduces 3 alerts when merging cd656d227890d7fe278394754cf59fbcc14bac40 into 96cb359 - view on LGTM.com

new alerts:

2 for Unused local variable
1 for Unused import

lgtm-com · 2020-09-08T03:03:19Z

This pull request introduces 3 alerts when merging df9d02c107df521efce184d0d9aa594d6a28f117 into 96cb359 - view on LGTM.com

new alerts:

2 for Unused local variable
1 for Unused import

kktheballer · 2020-09-08T17:56:36Z

retest default please

kktheballer · 2020-09-08T18:25:58Z

retest this please

kktheballer · 2020-09-08T20:31:25Z

retest this please

kktheballer · 2020-09-11T17:08:26Z

retest this please

kktheballer · 2020-09-14T03:45:26Z

retest this please

lguohan · 2020-10-07T17:50:14Z

show/ecmp.py

+from collections import OrderedDict
+
+@click.group(cls=clicommon.AliasedGroup)
+def ecmp():


i feel the name should be fgnhg, whic is it ecmp?

lguohan · 2020-10-21T18:19:48Z

can you paste the coverage report results?

kktheballer · 2020-10-21T20:47:26Z

lguohan · 2020-10-27T21:28:18Z

tests/fgnhg_test.py

+
+
+
+class TestECMP(object):


just minor comments, better to be rename to TestFineGrainedNexthopGroup

1. show fg-nhg-hash-view (shows the current hash bucket view of fg nhg) 2. show fg-nhg-active-hops (shows which set of next-hops are active)

--> It is identical to show-nhg-hash-view w/ the addition of hashbucketsize as well as the nhg prefix

…state_db tables

kktheballer · 2020-11-26T16:06:18Z

retest this please

anish-n suggested changes Aug 18, 2020

View reviewed changes

lguohan requested changes Aug 18, 2020

View reviewed changes

lguohan reviewed Aug 19, 2020

View reviewed changes

kktheballer force-pushed the master branch from b295439 to cd656d2 Compare September 8, 2020 02:43

lguohan reviewed Oct 7, 2020

View reviewed changes

kktheballer force-pushed the master branch from 4b008c0 to fb2b67a Compare October 13, 2020 00:01

kktheballer requested a review from lguohan October 21, 2020 17:16

lguohan reviewed Oct 27, 2020

View reviewed changes

tests/fgnhg_test.py Outdated

class TestECMP(object):

Copy link

Contributor

lguohan Oct 27, 2020

Choose a reason for hiding this comment

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

just minor comments, better to be rename to TestFineGrainedNexthopGroup

lguohan previously approved these changes Oct 27, 2020

View reviewed changes

kktheballer added 5 commits October 28, 2020 09:47

Show FG_NHG CLI Commands Added

059a606

1. show fg-nhg-hash-view (shows the current hash bucket view of fg nhg) 2. show fg-nhg-active-hops (shows which set of next-hops are active)

Adding show-nhg-group command

78d9128

--> It is identical to show-nhg-hash-view w/ the addition of hashbucketsize as well as the nhg prefix

Fixing some indentations from the past two commits

2b05dd6

Separated to ECMP.py, added ECMP UTs, and updated mock config_db and …

1d93ecf

…state_db tables

importing ecmp.py from main.py

606d104

kktheballer added 5 commits October 28, 2020 09:47

Fixing typo in state_db.json and addressing lgtm warnings

92562b7

Increase UT coverage

61cf33f

Fixing indentation

3d483ef

TestCase Fixes and mock config_db adjustment

27c5f28

Minor changes to CLI output format and rename Test Class

f6e54af

kktheballer dismissed lguohan’s stale review via f6e54af October 28, 2020 16:49

kktheballer force-pushed the master branch from 48a67d7 to f6e54af Compare October 28, 2020 16:49

lguohan approved these changes Oct 29, 2020

View reviewed changes

lguohan merged commit 8af9aee into sonic-net:master Oct 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show FG_NHG CLI Commands Added #1056

Show FG_NHG CLI Commands Added #1056

kktheballer commented Aug 18, 2020

lgtm-com bot commented Aug 18, 2020

lgtm-com bot commented Aug 18, 2020

anish-n Aug 18, 2020

kktheballer Sep 8, 2020

anish-n Aug 18, 2020

kktheballer Sep 8, 2020

lgtm-com bot commented Aug 18, 2020

lguohan commented Aug 18, 2020

lguohan left a comment

lguohan commented Aug 19, 2020

lguohan Aug 19, 2020

lguohan Aug 19, 2020

kktheballer Sep 8, 2020

lguohan Aug 19, 2020

lgtm-com bot commented Sep 8, 2020

lgtm-com bot commented Sep 8, 2020

kktheballer commented Sep 8, 2020

kktheballer commented Sep 8, 2020

kktheballer commented Sep 8, 2020

kktheballer commented Sep 11, 2020

kktheballer commented Sep 14, 2020

lguohan Oct 7, 2020

lguohan commented Oct 21, 2020

kktheballer commented Oct 21, 2020

lguohan Oct 27, 2020

kktheballer commented Nov 26, 2020

		@@ -1917,6 +1917,210 @@ def mmu():
		click.echo(proc.stdout.read())


		#

Show FG_NHG CLI Commands Added #1056

Show FG_NHG CLI Commands Added #1056

Conversation

kktheballer commented Aug 18, 2020

lgtm-com bot commented Aug 18, 2020

lgtm-com bot commented Aug 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Aug 18, 2020

lguohan commented Aug 18, 2020

lguohan left a comment

Choose a reason for hiding this comment

lguohan commented Aug 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Sep 8, 2020

lgtm-com bot commented Sep 8, 2020

kktheballer commented Sep 8, 2020

kktheballer commented Sep 8, 2020

kktheballer commented Sep 8, 2020

kktheballer commented Sep 11, 2020

kktheballer commented Sep 14, 2020

Choose a reason for hiding this comment

lguohan commented Oct 21, 2020

kktheballer commented Oct 21, 2020

Choose a reason for hiding this comment

kktheballer commented Nov 26, 2020