UPC / mailtoticket

Una pasarel·la de correu per al gestor de tiquets GN6.
GNU Affero General Public License v3.0
6 stars 7 forks source link

Repensar com les classes reben els valors de la configuració #125

Open alexm opened 8 years ago

alexm commented 8 years ago

Tal com hem parlat a #124 caldria repensar com fem que les diferents classes obtinguin els valors de la configuració.

Tasques:

alexm commented 8 years ago

Una primera proposta apunta a tenir les opcions de configuració com a paràmetres dels constructors de les classes que les necessiten.

Vegeu l'exemple proposat a https://github.com/UPC/mailtoticket/pull/124#issuecomment-203142833

aaguilera commented 8 years ago

Actualment el mòdul settings s'accedeix des dels següents fitxers:

correu.py
filtres/__init__.py
filtres/nou_extern.py
filtres/reply.py
filtres/nou.py
mailticket.py
mailtoticket.py
soa/tiquets.py
soa/identitat.py
soa/service.py
test/test_import.py
test/test_import.py
test/test_mailticket.py

Jo crec que, en la majoria de casos (suposo que en tots, però no m'he mirat cas per cas), no hi ha necessitat d'accedir directament a mòdul settings, sinó que es poden passar com a paràmetres del constructor únicament aquells atributs que siguin estrictament necessaris.

jaumemoral commented 8 years ago

Per reflexionar...

Els filtres es creen instanciant classes a partir del nom. Com sabem quins paràmetres hem de passar?

Una opció intermitja seria passar tot un objecte settings com a paràmetre al constructor. Així no tindrien el settings com a global. Però no sé com ho veieu... El 30/3/2016 19:06, "Angel Aguilera" notifications@github.com escribió:

Actualment el mòdul settings s'accedeix des dels següents fitxers:

correu.py filtres/init.py filtres/nou_extern.py filtres/reply.py filtres/nou.py mailticket.py mailtoticket.py soa/tiquets.py soa/identitat.py soa/service.py test/test_import.py test/test_import.py test/test_mailticket.py

Jo crec que, en la majoria de casos (suposo que en tots, però no m'he mirat cas per cas), no hi ha necessitat d'accedir directament a mòdul settings, sinó que es poden passar com a paràmetres del constructor únicament aquells atributs que siguin estrictament necessaris.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/UPC/mailtoticket/issues/125#issuecomment-203529948

aaguilera commented 8 years ago

Quant al tema dels filtres, el problema, tal i com jo ho veig, és que li estariem passant tots els "settings" als constructors, però cada filtre consultaria només uns quants atributs, que són els que tenen sentit per a aquella classe. Això no m'acaba de fer el pes, però d'entrada podriem considerar-ho acceptable, ja que ens facilitaria molt el tema dels tests, que de moment, entenc, és el principal motiu de replantejar-nos aquesta refactorització.

Si ens vulguéssim llençar a la piscina, la solució més "object-oriented" que se m'acudeix (no tan KISS), seria reorganitzar el fitxer de configuració, i associar a cada filtre els atributs que necessita, per exemple, en comptes de:

"filtres": [
    "filtres.reply.FiltreReply",
    "filtres.nou.FiltreNou"
],

podriem tenir:

"filtres": [
    { 
      "filterClass": "filtres.reply.FiltreReply", 
      "initParams": {
          "regex_reply": ".*?R[eEvV]:.*?\[Suport Unitat ([\d]+)\]", 
          "regex_privat": "(?i)\(comentari privat\)", 
          ... }
    },
    { 
      "filterClass": "filtres.nou.FiltreNou", 
      "initParams": {
            "equip_resolutor_nous": 11111,
            ... } 
    },
],

M'he inspirat en com es defineixen els filtres en els Deployment Descriptors de Java (p.ex. https://docs.oracle.com/cd/E13222_01/wls/docs81/webapp/web_xml.html#1015950). Després, podriem tenir una classe FilterFactory que cridariem més o menys així:

filtre = FilterFactory.newFilter(filterClass, initParams)

per a cada filtre que haguem indicat al fitxer de configuració.

jaumemoral commented 8 years ago

Jo havia arribat a pensar algo aixi, pero pot haver-hi parametres compartits entre diferents filtres. No ho acabo de veure clar, crec que complicaria massa el fitxer de configuració, que s'hauria de mantenir el més simple possible (de fet, ni tan sols tinc clar que hagi de ser un fitxer python, li dona massa "poder")

alexm commented 8 years ago

Passar l'objecte settings sencer em sembla una primer aproximació acceptable que podem anar refinant més endavant tal com apunta l'Àngel. Però el debat realment interessant és el del fitxer de configuració: crec que utilitzar un altre format (tipus ini amb una secció per classe, per exemple; o un yaml amb una clau per classe també) seria una bona forma d'enfocar el problema des d'un altre angle.

En general qualsevol cosa global, fins i tot els singletons, són una font de problemes, especialment amb els tests. Per això trobo molt interessant aquest debat. Endavant!