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

feat(huey): add Huey Integration #1555

Merged
merged 22 commits into from Jan 25, 2023

Conversation

Zhenay
Copy link
Contributor

@Zhenay Zhenay commented Aug 10, 2022

Add support for the Huey Task Queue https://huey.readthedocs.io/en/latest/

See #195

@Zhenay Zhenay force-pushed the Zhenay/195-huey-integration branch from 4a9dd8f to 1f76978 Compare August 11, 2022 21:17
@sl0thentr0py sl0thentr0py self-requested a review August 12, 2022 08:52
@sl0thentr0py sl0thentr0py added Status: Backlog enhancement New feature or request labels Aug 12, 2022
@Zhenay
Copy link
Contributor Author

Zhenay commented Nov 6, 2022

@sl0thentr0py, @antonpirker any plans to get this reviewed?

@antonpirker
Copy link
Member

Hey @Zhenay !
We are done reviewing a PyMongo integration and now is the time for Huey!
I will probably have a look at this later this week

@antonpirker antonpirker self-assigned this Nov 7, 2022
@antonpirker antonpirker added the New Integration Integrating with a new framework or library label Nov 7, 2022
@j-krl
Copy link

j-krl commented Nov 8, 2022

Looking forward to using this!

@sl0thentr0py sl0thentr0py requested review from antonpirker and removed request for sl0thentr0py November 8, 2022 14:22
@vercel
Copy link

vercel bot commented Nov 10, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
sentry-python ✅ Ready (Inspect) Visit Preview Nov 16, 2022 at 6:38PM (UTC)

@antonpirker
Copy link
Member

Hey @Zhenay !
First thing: amazing work, thank you soo much!

Could you please follow the steps in this error message here to make the PR ready for testing: https://github.com/getsentry/sentry-python/actions/runs/3437030024/jobs/5731266278

Thanks!

antonpirker
antonpirker previously approved these changes Nov 10, 2022
Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Just some cosmetic changes. If those are made and all the tests are green I will create a sample huey setup and give it a spin. (Tomorrow Friday, I am not in front of a computer, so I will be back here on Monday)

sentry_sdk/integrations/huey.py Outdated Show resolved Hide resolved
sentry_sdk/integrations/huey.py Outdated Show resolved Hide resolved
sentry_sdk/integrations/huey.py Outdated Show resolved Hide resolved
tests/integrations/huey/test_huey.py Outdated Show resolved Hide resolved
@antonpirker antonpirker dismissed their stale review November 10, 2022 13:24

did not want to approve but to request changes.

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Before I clicked wrong. So please make the changes mentioned in the comments in the PR.

When the changes are made and all the tests are green I will setup a sample huey project and give it a spin.
(I am not in front of a keyboard tomorrow (Friday) so I will be back here on Monday.)

@Zhenay
Copy link
Contributor Author

Zhenay commented Nov 18, 2022

Hey @antonpirker
Could you please run the workflow

@antonpirker
Copy link
Member

Hey @Zhenay ! I have now set up a simple demo app and everything looks good.

I have the spans for the enqueuing/submitting of tasks:
Screenshot 2023-01-25 at 13 09 38

And also the span for the task itself
Screenshot 2023-01-25 at 13 10 08

I added a little check in your branch to only send sensitive date (the arguments and kwargs of the task) when the option "send_default_pii" is set to true. Because there could be all sorts of sensitive data (passwords, email adresses, names, ... in there) and this data should never be sent by default.

@antonpirker
Copy link
Member

Hey @Zhenay !
Great work, everything looks good.

In your opinion, is this ready to be merged and shipped?
When there are some bug reports, will you be able to address them?

FYI: The next release will be in around 2 weeks or so.

Oh, and if you send me your shipping address (and t-shirt size) I can see If I can send you some swag as a little thank you. (send it to anton,pirker@sentry.io)

@Zhenay
Copy link
Contributor Author

Zhenay commented Jan 25, 2023

Hey, @antonpirker!
Thanks for your attention.

I suppose it can be merged. If any bugs are reported, I will be glad to fix them.

@antonpirker antonpirker merged commit 762557a into getsentry:master Jan 25, 2023
@antonpirker
Copy link
Member

Here you go! Thanks again!

@ulgens
Copy link

ulgens commented Feb 13, 2023

I couldn't find any reference to huey under https://docs.sentry.io/platforms/python/guides/django/configuration/integrations/, is it normal?

@nick4333
Copy link

nick4333 commented Jul 7, 2023

Are there any plans to update the documentation to include a huey integration?

@sentrivana
Copy link
Contributor

Hey @ulgens, @nick4333, sorry for the late reply, I've created an issue for the missing docs #2436 and we'll add them shortly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request New Integration Integrating with a new framework or library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants