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

fix(locking): Fix flaky test + Remove sentry excluded exception #2365

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

aminedhobb
Copy link
Collaborator

@aminedhobb aminedhobb commented Oct 1, 2024

Cette PR fait suite à #2340 et je modifie deux choses par rapport au commit initial:

  • Je change la spec du concern locked_jobs qui était flaky parce que le lock pouvait persister entre les 2 tests que comprend le fichier locked_jobs_spec.rb. Je fais en sorte d'englober chaque thread dans un bloc que l'on passe à ActiveRecord::Base.connection_pool.with_connection pour être sûr que le thread release sa connection (et donc le lock) après son exécution. Ce changement se trouve ici: 7aca12a

  • Suite à ce commentaire feat(app): Keep resources synched with rdvsp #2340 (comment) je remets l'exception sur sentry le temps de voir si on a bien des jobs qui raise en tentant d'acquérir le lock. Ce changement se trouve ici: 97ee7c8

@aminedhobb aminedhobb requested a review from Holist October 1, 2024 09:18
@aminedhobb aminedhobb self-assigned this Oct 1, 2024
Holist
Holist previously approved these changes Oct 1, 2024
Copy link
Collaborator

@Holist Holist left a comment

Choose a reason for hiding this comment

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

🙇
merci Amine

@aminedhobb aminedhobb changed the title fix(sentry): Remove excluded exception fix(locking): Fix flaky test + Remove sentry excluded exception Oct 1, 2024
@aminedhobb aminedhobb force-pushed the fix/remove-excluded-sentry-exception branch from 5c1ec79 to 7aca12a Compare October 1, 2024 12:42
Copy link
Collaborator

@Holist Holist left a comment

Choose a reason for hiding this comment

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

lgtm.
Merci !

@aminedhobb aminedhobb merged commit d8ca592 into staging Oct 1, 2024
7 checks passed
@aminedhobb aminedhobb deleted the fix/remove-excluded-sentry-exception branch October 1, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants