Jsonify show neighbor,adj-rib-in,adj-rib-out cmds output #1240

vish-321 · 2024-10-16T15:07:56Z

This PR updates ExaBGP to optionally provide JSON output for API commands such as show neighbor, adj-rib-in, and adj-rib-out.

Existing Commands:

show neighbor [optional neighbor ip] summary
show neighbor [optional neighbor ip] extensive
show neighbor [optional neighbor ip] configuration
show adj-rib in [extensive] (to be integrated with neighbor)
show adj-rib out [extensive] (to be integrated with neighbor)

Example Usage for JSON Output:

show neighbor [optional json] [optional neighbor ip] summary
show neighbor [optional json] [optional neighbor ip] extensive
show adj-rib in [optional json] [extensive] (to be integrated with neighbor)
show adj-rib out [optional json] [extensive] (to be integrated with neighbor)

Examples:

show neighbor json summary
show neighbor json extensive
show adj-rib out json

This should also resolve the issue detailed in #909

thomas-mangin · 2024-10-16T17:00:37Z

src/exabgp/reactor/api/command/rib.py

@@ -7,6 +7,8 @@
 License: 3-clause BSD. (See the COPYRIGHT file)
 """

+import json


I do not believe this import is required.

thomas-mangin · 2024-10-16T17:02:43Z

src/exabgp/bgp/neighbor.py

+        }
+
+        if json:
+            return data


This code seems to return a dict and not a string, it should be a string. While python is dynamic, I attempt to only return one type per function, like in C, to ease debugging.

thomas-mangin · 2024-10-16T17:03:08Z

src/exabgp/reactor/api/command/neighbor.py

 class Neighbor(object):
    extensive_kv = '   %-20s %15s %15s %15s'
    extensive_template = """\
 Neighbor %(peer-address)s

-    Session                         Local
+	Session                         Local


It looks like spaces have been converted to tabs

thomas-mangin · 2024-10-16T17:04:33Z

src/exabgp/reactor/api/command/neighbor.py

-
-    @classmethod
-    def extensive(cls, answer):
+    def extensive(cls, answer, output_format='text'):


extensive is unlikely to be right as a @classmethod, it is a normal one class function.

One for me to look into if the previous code was already a @classmethod

However, using an output_format as a string, not a boolean or an "enum", is wrong. String should not be given magical if it is possible to compare them.

thomas-mangin · 2024-10-16T17:05:42Z

src/exabgp/reactor/api/command/neighbor.py

@@ -73,99 +63,98 @@ class Neighbor(object):
    summary_template = '%-15s %-7s %9s %-12s %10d %10d'

    @classmethod
-    def as_dict(cls, answer):


as_dict should not be inlined. It would be better to have as _string added and use two functions for clarity.

thomas-mangin · 2024-10-16T17:06:57Z

src/exabgp/reactor/api/command/neighbor.py

@@ -194,22 +183,23 @@ def teardown(self, reactor, service, line):
        return False


-@Command.register('text', 'show neighbor', False, ['summary', 'extensive', 'configuration', 'json'])
+@Command.register('text', 'show neighbor', False, ['summary', 'extensive', 'configuration'])
+@Command.register('text', 'show neighbor json', False, ['summary', 'extensive'])


if we register json, we should also register text and make it work.

thomas-mangin · 2024-10-16T17:08:12Z

src/exabgp/reactor/loop.py

        peer = self._peers.get(peer_name, None)
        if not peer:
            log.critical('could not find referenced peer', 'reactor')
            return ""
-        return peer.neighbor.name()
+        if json:


The if statement is not required, return peer.neighbor.name(json) does the same thing AFAICS.

thomas-mangin · 2024-10-16T17:08:49Z

src/exabgp/reactor/loop.py

@@ -172,12 +172,16 @@ def neighbor(self, peer_name):
            return
        return peer.neighbor

-    def neighbor_name(self, peer_name):
+    def neighbor_name(self, peer_name, json=False):


json should be made mandatory and not defaulted to False

thomas-mangin · 2024-10-16T17:11:00Z

src/exabgp/reactor/api/command/rib.py

    try:
        rib = words[2]
-        if not rib in ('in', 'out'):
+        if not rib in ('in', 'out', 'json'):


This logic seems wrong ... I would need to look more in depth to say why but I can not ATM.

For clarity it is also about the if rib == 'json' part too.

thomas-mangin · 2024-10-16T17:12:01Z

Thank you for this PR, I can not accept it as it but I have no objection to adding the feature.

Jsonify show neighbor,adj-rib-in,adj-rib-out cmds output

7f217a6

thomas-mangin reviewed Oct 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jsonify show neighbor,adj-rib-in,adj-rib-out cmds output #1240

Jsonify show neighbor,adj-rib-in,adj-rib-out cmds output #1240

vish-321 commented Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin Oct 16, 2024

thomas-mangin commented Oct 16, 2024

Jsonify show neighbor,adj-rib-in,adj-rib-out cmds output #1240

Are you sure you want to change the base?

Jsonify show neighbor,adj-rib-in,adj-rib-out cmds output #1240

Conversation

vish-321 commented Oct 16, 2024

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

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

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

thomas-mangin commented Oct 16, 2024