Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configuration du mailer + messages asynchrones #1160

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Conversation

mmarchois
Copy link
Collaborator

@mmarchois mmarchois commented Jan 21, 2025

Cette PR rajoute la configuration du mailer ainsi que la gestion asynchrone des messages, qui sera utilisée pour l'envoi d'email.

Todo :

  • En attente d'un compte Brevo

@codecov-commenter
Copy link

codecov-commenter commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.67%. Comparing base (97f7ab1) to head (3467262).

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1160   +/-   ##
=========================================
  Coverage     98.67%   98.67%           
  Complexity     1852     1852           
=========================================
  Files           368      368           
  Lines          8014     8014           
=========================================
  Hits           7908     7908           
  Misses          106      106           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mmarchois mmarchois self-assigned this Jan 21, 2025
@mmarchois mmarchois marked this pull request as ready for review January 21, 2025 16:27
@mmarchois mmarchois requested review from florimondmanca, AntoineAugusti and Lealefoulon and removed request for AntoineAugusti January 21, 2025 16:27
Copy link
Collaborator

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Juste une petite question

Et, est-ce qu'il y a une manip que je peux faire en local pour vérifier que tout est bien configuré ?

@mmarchois mmarchois force-pushed the feat/async-mailer branch 2 times, most recently from 240037e to 1b0df29 Compare January 23, 2025 09:00
Copy link
Collaborator

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'approuve en avance (par rapport au compte Brevo en attente)

@mmarchois
Copy link
Collaborator Author

mmarchois commented Jan 23, 2025

@florimondmanca suite à ton commentaire, j'ai repensé un peu le système; J'ai rajouté redis commander, qui permet de voir les messages (notamment ceux en erreur, dans une interface web) et j'ai mis supervisor pour gérer nos workers.

Aussi, le fait que Brevo soit en attente n'est pas bloquant pour merger cette PR. C'est juste un reminder

@florimondmanca
Copy link
Collaborator

florimondmanca commented Jan 23, 2025

j'ai repensé un peu le système; J'ai rajouté redis commander, qui permet de voir les messages (notamment ceux en erreur, dans une interface web) et j'ai mis supervisor pour gérer nos workers.

@mmarchois OK 👍 Je ne connaissais pas Redis Commander. Est-ce que ça vaut le coup de mettre à jour docs/tools/redis.md ? Je viens de voir que tu l'as déjà fait 👍

@mmarchois mmarchois merged commit b55c87a into main Jan 23, 2025
6 checks passed
@mmarchois mmarchois deleted the feat/async-mailer branch January 23, 2025 13:20
Comment on lines +298 to +308
## ----------------
## Supervision
## ----------------
##

supervisor_status:
docker compose exec supervisor supervisorctl status

supervisor_restart:
docker compose exec supervisor supervisorctl restart messenger-worker:*

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je chipote peut-être mais, puisque Supervisor est utilisé pour démarrer le worker messenger en local, quid de le traiter comme un détail d'implémentation ?

Suggested change
## ----------------
## Supervision
## ----------------
##
supervisor_status:
docker compose exec supervisor supervisorctl status
supervisor_restart:
docker compose exec supervisor supervisorctl restart messenger-worker:*
## ----------------
## Workers (for local development)
## ----------------
##
workers_status:
docker compose exec supervisor supervisorctl status
workers_restart:
docker compose exec supervisor supervisorctl restart messenger-worker:*

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok je traiterai ton retour dans la prochaine PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants