#184 Log des attributions d'ip

Bumpitybump,

any feedback on this ?

Bumpitybump, est-ce que quelqu'un a du feedback sur cette PR ? :s

(Je #harcele un peu, mais on aimerai remettre à plat notre COIN avant la fin de l'année ^^')

Merci pour les relances, j'essaye très fort de caler ça à un moment dans les semaines à venir :-).

Test

Ça fonctionne :-) (testé sur ajout/suppression/modif)

Changements à apporter ?

Code sans rapport avec la PR ?

+@receiver(post_save, sender=OfferSubscription)
+def offer_subscription_event(sender, **kwargs):
+    os = kwargs['instance']
+
+    if not hasattr(os, 'configuration'):
+        config_cls = None
+        for subconfig_cls in Configuration.__subclasses__():
+            if subconfig_cls().__class__.__name__ == os.offer.configuration_type:
+                config_cls = subconfig_cls
+                break
+
+        if config_cls is not None:
+            config = config_cls.objects.create(offersubscription=os)
+            for offer_ip_pool in os.offer.offerippool_set.order_by('-to_assign'):
+                IPSubnet.objects.create(
+                                configuration=config,
+                                ip_pool=offer_ip_pool.ip_pool)
+            config.save()
+

Ça ça n'a rien à voir avec la PR ça, si ?

Informations personelles

+IP_ALLOCATION_MESSAGE = "{ip} to {member.pk} ({member.username} - {member.first_name} {member.last_name}) (for offer {offer}, {ref})"

Je pense que l'on ne devrait pas avoir une config par défaut qui logue nom et prénom des personnes. Cela rend impossible le respect cadre réglementaire qui demande de supprimer ces infos personnelles 1 an exactement après la fin d'un contrat (c'est hors de portée pour logrotate ça demanderait de supprimer les données sélectivement).

Qu'ARN fasse ce choix de conf, ça regarde ARN, mais je pense que l'on doit éviter à tout prix ça dans la conf par défaut de coin.

Logging en debug

1f01953 * Log as debug because for some weird reason, during tests info messages are not logged x_x

Deux remarques :

• je n'ai pas reproduit ce comportement fautif dont tu parles (.info log comme il faut lors de mes tests)
• la conf par défaut des loggers (commit 4506f47) est au niveau info et non debug (donc par défaut, rien n'est logué).

Message de log

J'ai bien lu le choix fait (et justifié) d'avoir le même traitement à la création et à l'update. Mais je pense que c'est trompeur au niveau des logs. Aussi, sans changer le code, je pense que l'on pourrait Écrire [Allocating or updating IP] plutôt que [Allocationg IP].

Rebase et squash

Car ça date un peu (mea culpa) :-)

Oyo ! Thx for teh feedback !

Code sans rapport avec la PR ?

Eh ça fait genre 1 an que j'ai fait ça alors je t'avoue que je sais plus pourquoi j'ai écrit ça, d'autant que les abonnement et IPsubnet c'est pas ce que je comprends le plus dans coin x_x.

Naivement en relisant le code je dirais que si, ça m'a l'air lié car il y a un IPSubnet.objects.create qui est succeptible ensuite de trigger subnet_event_save et donc une ligne dans le code ... mais je saurais pas te dire pourquoi tout ce pâté :|

Informations personelles

Agreed. Du coup, j'enlève juste - {member.first_name} {member.last_name} ? (Pas sur de savoir pourquoi on a choisi de faire comme ça dans ARN)

Logging en debug

Ugh beh yep, me suis ptete trompé dans mes tests. Si tu confirmes que ça pose pas de soucis de remettre ça en info alors go, mais je t'avoue que j'ai un peu la flemme de refaire le test de mon côté :D.

Message de log

Agreed ;)

Rebase et squash

Yis

Will do all that when I have some energy

Eh ça fait genre 1 an que j'ai fait ça alors je t'avoue que je sais plus pourquoi j'ai écrit ça, d'autant que les abonnement et IPsubnet c'est pas ce que je comprends le plus dans coin x_x.

OK, je comprends. À lire le code ça semble être l'attribution auto de conf et donc d'IP à la sauvegarde de l'abonnement je pense.

J'avoue que j'ai un peu peur des effets de bord faute de tests, mais si c'est trop loin pour toi/vous… bref, on va avancer vers le merge et tant pis :-).À la limite, par un coup de git-fu est-ce que tu peux juste bouger ça dans un commit à part du reste, ça rendra au moins lisible l'intention :-).

OK pour tout le reste (et oui je confirme que ça marche pour le log en info, chez moi en tout cas).

Will do all that when I have some energy

Ok, take care and time to battery-charge yourself well ;-)