[spp] [PATCH 0/2] Python 3 support for topo command

[spp] [PATCH 0/2] Python 3 support for topo command

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

Hi,

This update is to fix bugs of topo command.

* Topo command is failed if 'imgcat.sh' which is required only for
  MacOS does not exist. Linux user uses `img2sixel' instead of it.
  It is fixed by changing not to find 'imgcat.sh' if 'img2sixel'
  exits.

* Topo is not update for python 3 and cause an error of encoding.
  To avoid this error, add encoding appropriately.


Yasufumi Ogawa (2):
  controller: fix bug of topo to refer external cmd
  controller: fix encoding in topo command

 docs/guides/commands/experimental.rst |  2 +-
 src/controller/topo.py                | 32 ++++++++++++++++++---------
 2 files changed, 23 insertions(+), 11 deletions(-)

-- 
2.17.1

[spp] [PATCH 1/2] controller: fix bug of topo to refer external cmd

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

'topo term' command uses img2sixel (or imgcat for iTerm2) for generating
an image of topology, but there is no checking if the command exists.

This update is to add checking it before generating image. If the
command is not existing, 'topo term' shows an error message and do
nothing.

This update is also including an change of the name of imgcat script
from 'imgcat.sh' to 'imgcat'. It is just to simplify setting up.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 docs/guides/commands/experimental.rst |  2 +-
 src/controller/topo.py                | 25 ++++++++++++++++++-------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/docs/guides/commands/experimental.rst b/docs/guides/commands/experimental.rst
index 8bb8e20..a678fe6 100644
--- a/docs/guides/commands/experimental.rst
+++ b/docs/guides/commands/experimental.rst
@@ -81,7 +81,7 @@ If you use iTerm2, you have to get a shell script
 ``imgcat`` from `iTerm2's displaying support site
 <https://iterm2.com/documentation-images.html>`_
 and save this script as
-``spp/src/controller/3rd_party/imgcat.sh``.
+``spp/src/controller/3rd_party/imgcat``.
 
 .. _figure_topo_term_exp:
 
diff --git a/src/controller/topo.py b/src/controller/topo.py
index c6347a4..d578290 100644
--- a/src/controller/topo.py
+++ b/src/controller/topo.py
@@ -270,14 +270,25 @@ class Topo(object):
         if spawn.find_executable("img2sixel") is not None:
             img_cmd = "img2sixel"
         else:
-            img_cmd = "%s/%s/imgcat.sh" % (
+            imgcat = "%s/%s/imgcat" % (
                 os.path.dirname(__file__), '3rd_party')
-        # Resize image to fit the terminal
-        img_size = size
-        cmd = "convert -resize %s %s %s" % (img_size, tmpfile, tmpfile)
-        subprocess.call(cmd, shell=True)
-        subprocess.call("%s %s" % (img_cmd, tmpfile), shell=True)
-        subprocess.call(["rm", "-f", tmpfile])
+            if os.path.exists(imgcat) is True:
+                img_cmd = imgcat
+            else:
+                img_cmd = None
+
+        if img_cmd is not None:
+            # Resize image to fit the terminal
+            img_size = size
+            cmd = "convert -resize %s %s %s" % (img_size, tmpfile, tmpfile)
+            subprocess.call(cmd, shell=True)
+            subprocess.call("%s %s" % (img_cmd, tmpfile), shell=True)
+            subprocess.call(["rm", "-f", tmpfile])
+        else:
+            print("img2sixel (or imgcat.sh for MacOS) not found!")
+            topo_doc = "https://spp.readthedocs.io/en/latest/"
+            topo_doc += "commands/experimental.html"
+            print("See '%s' for required packages." % topo_doc)
 
     def format_sec_status(self, sec_id, stat):
         """Return formatted secondary status as a hash
-- 
2.17.1

[spp] [PATCH 2/2] controller: fix encoding in topo command

[ogawa.yasufumi]

From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>

For python3, it is required to treat str and bytes explicitly for
sending and receiving messages via socket.

This update is for encoding messages to 'utf-8' similar to previous
commit '7c5193f'.

Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
---
 src/controller/topo.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/controller/topo.py b/src/controller/topo.py
index d578290..e608961 100644
--- a/src/controller/topo.py
+++ b/src/controller/topo.py
@@ -32,7 +32,8 @@ class Topo(object):
     def show(self, dtype, size):
         res_ary = []
         for sec_id in self.sec_ids:
-            self.m2s_queues[sec_id].put("status")
+            msg = "status"
+            self.m2s_queues[sec_id].put(msg.encode('utf-8'))
             res = self.format_sec_status(
                 sec_id, self.s2m_queues[sec_id].get(True))
             res_ary.append(res)
@@ -47,7 +48,8 @@ class Topo(object):
     def output(self, fname, ftype="dot"):
         res_ary = []
         for sec_id in self.sec_ids:
-            self.m2s_queues[sec_id].put("status")
+            msg = "status"
+            self.m2s_queues[sec_id].put(msg.encode('utf-8'))
             res = self.format_sec_status(
                 sec_id, self.s2m_queues[sec_id].get(True))
             res_ary.append(res)
@@ -266,7 +268,6 @@ class Topo(object):
         from distutils import spawn
 
         # TODO(yasufum) Add check for using only supported terminal
-
         if spawn.find_executable("img2sixel") is not None:
             img_cmd = "img2sixel"
         else:
-- 
2.17.1

Re: [spp] [PATCH 0/2] Python 3 support for topo command

[Ferruh Yigit]

On 6/12/2018 8:03 AM, ogawa.yasufumi@lab.ntt.co.jp wrote:
> From: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
> 
> Hi,
> 
> This update is to fix bugs of topo command.
> 
> * Topo command is failed if 'imgcat.sh' which is required only for
>   MacOS does not exist. Linux user uses `img2sixel' instead of it.
>   It is fixed by changing not to find 'imgcat.sh' if 'img2sixel'
>   exits.
> 
> * Topo is not update for python 3 and cause an error of encoding.
>   To avoid this error, add encoding appropriately.
> 
> 
> Yasufumi Ogawa (2):
>   controller: fix bug of topo to refer external cmd
>   controller: fix encoding in topo command

Series applied, thanks.