-
Notifications
You must be signed in to change notification settings - Fork 20
Add sentry to our task queues #1160
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
Conversation
bklvsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move sentry_sdk.init() to albs.dramatiq.init.py file so that the code isn't duplicated across all actors?
@bklvsky Yes, I had troubles with circular import here last time, but it seems fine for this one |
bklvsky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
javihernandez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a couple of recommendations for better git commit messages from our commit guidelines:
- The reference to the issue being resolved should be mentioned (if any) in the first commit message
- "Remove duplication" as a commit message can be a bit ambiguous. You could probably add some more meaningful commit message like "Remove sentry setup duplication" or "Remove sentry setup duplication in task queues". Up to you.
If you want to fix the commit messages, you can, from your current sentry-dramatiq branch, do the following:
git format-patch HEAD^^# this will generate 2 patchesgit reset HEAD^^ --hard# this will reset your branch (and reset staged changes)- At this point, you can either:
- update your two patches directly to fix the commit messages. And then
git amboth patches git amthe first patch, do agit commit --amendand fix the commit message. repeat for the second one
git push <your_origin_name> sentry-dramatiq --forceto update your branch with fixed commits
979e6c8 to
ef542dc
Compare
Resolves: AlmaLinux/build-system#489