reingart / pyafipws

Factura Electrónica AFIP y otros servicios web (proyecto software libre) — Interfases, tools and apps for Argentina's gov't. webservices (soap, com/dll simil-ocx, pdf, dbf, xml, json, etc.) #python
http://www.sistemasagiles.com.ar/trac/wiki/ManualPyAfipWs
GNU Lesser General Public License v3.0
286 stars 224 forks source link

Corrección al importar pythoncom en plataformas != Windows #77

Closed cperezabo closed 4 years ago

cperezabo commented 4 years ago

La mayoría de imports de pythocom a lo largo del código se dan dentro del argumento --register, por lo cual se pueden considerar "conscientes"

El problema es que en el caso empaquetar la aplicación para macOS (pasaría lo mismo en Linux) da problemas, porque no esta corriendo en Windows, y mucho menos siendo empaquetada con py2exe

cperezabo commented 4 years ago

Pensando bien esto, pythoncom no está siquiera listado en requirements.txt, por lo que si alguien empaqueta para Windows sin instalar pythoncom, termina recibiendo un error de modulo inexistente y resulta molesto manejar dependencias innecesarias.

reingart commented 4 years ago

Hola @cperezabo , gracias por la contribución!

Esto en Linux como te llega a pasar? Si recuerdo bien, el código en cuestión solo se ejecutaría si está "compilado" con py2exe (sys.frozen no debería existir en otras condiciones)

pythoncom (pywin32) no esta en el requirements porque solo aplica si se quiere compilar los objetos ActiveX (COM) para accederlos desde otros lenguajes

Si tenes un traceback avisame y lo vemos

cperezabo commented 4 years ago

En principio pienso que si solo aplica a entornos ActiveX entonces esta modificación es necesaria, ya que en mi caso he empaquetado para Windows y siempre he tenido que comentar estas líneas para evitar tener que instalar y distribuir innecesariamente pythoncom.

Por otro lado, también estoy empaquetando para macOS y en ese entorno es innecesario absolutamente. En Linux pasaría lo mismo.

En su momento usaba py2exe para Windows y py2app para macOS, pero hace unos años que unifiqué ambos procesos con PyInstaller, y en todos los casos el atributo frozen es establecido.

En el caso de PyInstaller, frozen se establecía a False en versiones anteriores y ahora a True según la documentación, por lo que validar con hasattr como se hace actualmente devuelve True, siendo este un falso positivo porque no se está utilizando py2exe ni corriendo en Windows. py2app también establece frozen y hasta donde se todos lo hacen de una u otra forma. De hecho es lo que siempre he usado para comprobar si estoy en un bundle o no.

Por estas razones creo que lo más óptimo es meterlo en un try, en lugar de tener que hacer validaciones para los posibles valores que se le puedan atribuir a frozen en cada uno de los posibles empaquetadores que se utilicen.

En Windows uno podría bypassear esto instalando pythoncom, pero en macOS o Linux ese paquete no es siquiera instalable! Por lo que hay que comentar estas líneas como dije más arriba, y hacerlo cada vez que se actualiza pyafipws. Un dolor de cabeza.

¿Que pensás?

cperezabo commented 4 years ago

Hola @reingart, aprovecho el PR para agregar un commit (https://github.com/reingart/pyafipws/pull/77/commits/f2e427f37e8bea0fb1193eaa51e762d38eef3b94) con un fix de encoding surgido a base de https://github.com/pysimplesoap/pysimplesoap/pull/187

Al corregir el código de PySimpleSOAP para que los tests pasen, aparecieron algunos bugs en PyAfipWs derivados del pase de str/unicode a bytes/str

Entonces SimpleXMLElement.as_xml() devuelve str como era esperado según los tests existentes, y se transforma a bytes en IIBB para usar con hashlib.md5 y en WSAA con M2Crypto.BIO.MemoryBuffer

@lukio te meto a vos en la conversación para que estes al tanto.

lukio commented 4 years ago

Hola @cperezabo, como estas? Tengo dos temas para organizarnos mejor:

abrazo,

cperezabo commented 4 years ago

Ahora separo el commit en otra rama y creo un PR independiente. Con respecto al Issue sobre empaquetar en plataformas != Windows, les puedo confirmar que no hace falta mas que este cambio. Hace años que estoy empaquetando para macOS y Windows en simultaneo. Lo único que molesta es la incorrecta validación presentada en este PR para pythoncom que solo es necesario en Windows.

cperezabo commented 4 years ago

Listo, creé #80