Broker disponible para mastermind y commander

[tomasalmeida]

• [ ] dejar el codigo disponible para que se pueda usar en ambos lados
• [ ] crear un suscriptor para exhibir los mensajes
• [ ] definir los topics disponibles

Esta tarea es de formación por @jjmontesl, no necesariamente entra en el código

[jjmontesl]

Actualmente el código del broker está en en los directorios de ambos proyectos. En mi opinión, y por hacerlo simple, lo dejaría en /commander. Se pueden importar módulos entre ambos, no big deal.

Haría varios apuntes respecto al código del broker:

1) No es necesario que "encode" y "decode" sean funciones del módulo, podrían ser miembros privados de la clase y en ese caso, ¿por qué no incluir también el json.encode en ese método? de todas formas son métodos de una línea que no se reutilizarán, yo los eliminaría y pondría su código directamente allí donde se usen.

2) No necesitamos serializar nosotros: en la documentación sobre serialización de PyZMQ pone "A socket has the methods send_json() and send_pyobj(), which correspond to sending an object over the wire after serializing with json and pickle respectively, and any object sent via those methods can be reconstructed with the recv_json() and recv_pyobj() methods.". Usemos por tanto send_json y olvidemos el serialize y el base64.

3) Usar llamadas no-bloqueantes (peek) provocará que el proceso consuma toda la CPU disponible o tenga latencia. Los sistemas de mensajería trabajan alrededor de un "event-loop" (al igual que hacen Twisted o asyncio en Python o las interfaces Future y Executor en Java). De todas formas ya veremos esto ;).

4) Usar __del__ para hacer limpieza (o como en este caso, para parar un event loop) no es una buena idea, pero ya veremos esto también. Bórrese el método, ya nos preocuparemos de la limpieza ;).

5) Devolver "BOO" no es serio. Úsese una constante con un nombre y valor más descriptivo, o valor None. O láncese una excepción, o devuélvase un flag de estado "ok/error". De todas formas como dije arriba, el método "peek" es en general una mala idea: usamos una cola de mensajería para poder responder a los eventos exactamente en el momento que llegan, y luego pasamos de ella usando "peek" continuamente. La CPU tiene mejores cosas que hacer. Ya revisaremos esto con calma.

6) También estamos suscribiendo los topics múltiples veces (cada vez que se llama a peek). Debería hacerse sólo en la inicialización. Elimínese eso de "peek" y hágase en el constructor.

7) Las clases _Subscriber y _Publisher por algún motivo tienen un guión bajo delante (_). No es necesario. Si realmente creemos que deben ser privadas, pueden definirse dentro de Broker pero no es común. Tampoco aporta grandes beneficios considerar las clases privadas, ya que las clases sólo existen dentro de su módulo (hasta que se importan, claro). Por comparar, en Java tampoco existe el concepto de "clases privadas" por el mismo motivo.

Este módulo necesita un buen repaso de corta pega, pero el código está ahí. Vamos hablando.

[alexhermida]

@jjmontesl yo pienso que la clase/modulo son los suficientemente pequeña como para tener el código del suscriptor y el publicador en cada aplicación por separado. Al fin y al cabo es una clase mínima que envuelve zmq. Quizás si crece más tiene sentido o no porque las funciones sería distintas, pero por ahora yo simplemente haría un copia pega. ¿Cómo lo ves? Con respecto al resto de observaciones lo veo genial.

[jjmontesl]

A mí me parece ok el copiapega, además creo que cada clase consumidora (subscriber) debería registrar su propio callback.

Pero ahora al menos la clase no es tan mínima, y Tomás estaba interesado en el ejercicio de tener un módulo reutilizable, que tampoco es que sea una mala idea. Pero vale, hacemos uno, y si es realmente reutilizable, lo copiamos a la otra app.

Tampoco entiendo muy bien el código, la verdad, creo que le hace falta una revisión buena. No veo que se llegue a inicializar el ioloop de ZMQ (aunque se usan ZMQStream), así que el flujo del proceso no lo entiendo. Así que cuando le demos la siguiente vuelta supongo que decidiremos con qué forma se queda.

Tampoco sé si para añadir más funcionalidad se harían más apps/procesos (ya que está ZMQ ahí). Yo personalmente añadiría más módulos a Commander (data logging, lógica de control...) antes que más apps, pero para ello hay que empezar a hacer Commander realmente async o usar multiprocessing o threading. ¿Cómo se plantea la arquitectura cuando haya más componentes? aunque este issue no es el sitio adecuado para esta pregunta (¿si podéis comentar sobre esto en slack?).

[tomasalmeida]

1. done
2. send_json solamente pilla el json como parametro, como se envía diferentes topics? No acabo de comprender este caso. En el send, la primera palabra es el topic (yo uso un consumidor en node.js para probar la compatibilidad entre sistemas).
3. ok
4. done
5. método eliminado, no veo sentido, se hace un subscribe y desde ahí ya trabaja.
6. done
7. done

[tomasalmeida]

current on going changes: https://github.com/fanequinha/cerebro/compare/feature/add_more_info?expand=1