Development #41284
Ne pas ré-initialiser last_account_deletion_alert s'il est déjà à None
0%
Description
En dehors du fait que ça casse des tests dans hobo, c'est inutile.
Fichiers
Demandes liées
Révisions associées
misc: simplify logging in clean-unused-accounts (#41284)
misc: do no clear last_account_deletion_start on login (#41284)
Historique
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Lié à Development #26909: clean-unused-account doit dépendre d'une configuration lié à l'OU ajouté
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Fichier 0001-misc-update-last_account_deletion_alert-only-if-nece.patch 0001-misc-update-last_account_deletion_alert-only-if-nece.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
J'aurai préféré virer complètement ça et le gérer au niveau du cron d'envoi des mails mais ce sera pour une autre fois.
Mis à jour par Nicolas Roche il y a environ 4 ans
- Statut changé de Solution proposée à Solution validée
Dans authentic2/management/commands/clean-unused-accounts.py,
la fonction clean_unused_accounts() filtre les utilisateurs à supprimer/notifier
sur l'attribut last_login.
Donc pas besoin de remettre à None l'attribut last_account_deletion_alert à
chaque connexion pour ne pas supprimer l'utilisateur.
Pour moi, le test sur cet attribut pour envoyer une alerte uniquement aux utilisateurs
connectés depuis fait qu'à mon avis ce code n'est jamais exécuté.
users = User.objects.filter(ou=ou, last_login__lte=now-alert_delay) for user in users.filter(last_account_deletion_alert__isnull=True): self.send_alert(user)
(mais dans les faits il est couvert par les tests)
Mis à jour par Paul Marillonnet il y a environ 4 ans
Nicolas Roche a écrit :
Dans authentic2/management/commands/clean-unused-accounts.py,
la fonctionclean_unused_accounts()filtre les utilisateurs à supprimer/notifier
sur l'attributlast_login.
Donc pas besoin de remettre à None l'attribut last_account_deletion_alert à
chaque connexion pour ne pas supprimer l'utilisateur.
Ce n'est à mon avis pas suffisant. Je pense que tu as en tête une autre version des conditions de filtrage qui seraient :
for user in users.filter(last_account_deletion_alert__lte=F('last_login')):
self.send_alert(user)
auquel cas oui pas besoin de remettre à null ce champ
last_account_deletion_alert, mais ce n'est pas la direction empruntée par Valentin dans #26909, laquelle est à mon avis tout aussi viable. Je plussoie pour le ack, pousse ça Benj.Mis à jour par Valentin Deniaud il y a environ 4 ans
Paul Marillonnet a écrit :
Ce n'est à mon avis pas suffisant. Je pense que tu as en tête une autre version des conditions de filtrage qui seraient :
[...]
C'est vachement mieux comme patch, pousse ça sur une autre branche être sûr que ça build et Benj choisit celui qu'il pousse ?
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Fichier 0001-misc-style-PEP8-41284.patch 0001-misc-style-PEP8-41284.patch ajouté
- Fichier 0002-misc-do-not-update-last_account_deletion_alert-on-lo.patch 0002-misc-do-not-update-last_account_deletion_alert-on-lo.patch ajouté
- Statut changé de Solution validée à Solution proposée
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
J'ai passé les erreurs de mail en warning mais je peux revenir là dessus, c'est juste que ça fait des tonnes de mails d'erreur si y un souci avec le serveur de mail.
Mis à jour par Valentin Deniaud il y a environ 4 ans
Je préférerais nettement voir les fix sur le logging et même le nouveau self.now dans le premier patch, qu'il n'y ait dans le second que le correctif.
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Fichier 0001-misc-style-PEP8-41284.patch 0001-misc-style-PEP8-41284.patch ajouté
- Fichier 0002-misc-simplify-logging-in-clean-unused-accounts-41284.patch 0002-misc-simplify-logging-in-clean-unused-accounts-41284.patch ajouté
- Fichier 0003-misc-do-no-clear-last_account_deletion_start-on-logi.patch 0003-misc-do-no-clear-last_account_deletion_start-on-logi.patch ajouté
Voilà, j'ai encore simplifié les logs, on s'arrête à la première exception, si le SMTP ou autre chose ne marche pas on le saura tout de suite et on ne recevra pas 100 mails différents pour nous le dire, sur les échecs critiques comme cela je pense qu'il faut éviter de vouloir être trop malin.
Mis à jour par Valentin Deniaud il y a environ 4 ans
- Statut changé de Solution proposée à Solution validée
Ça roule.
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 7ab92c578f9b0bee5ae2208c34e87812863c9a91
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Fri Apr 3 12:20:07 2020 +0200
misc: do no clear last_account_deletion_start on login (#41284)
commit 426891a7a024452076fbd612700f5877048e8ef3
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Fri Apr 3 12:16:35 2020 +0200
misc: simplify logging in clean-unused-accounts (#41284)
commit f2f80d5f30727edaaae5acbc56ad43f062d6258d
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Thu Apr 2 22:03:17 2020 +0200
misc: style, PEP8 (#41284)Mis à jour par Paul Marillonnet il y a environ 4 ans
- Statut changé de Résolu (à déployer) à Solution déployée
misc: style, PEP8 (#41284)