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

1564 Add logging for failed subscriber creation #1565

Merged
merged 15 commits into from
Oct 31, 2024

Conversation

dhochbaum-dcp
Copy link
Collaborator

@dhochbaum-dcp dhochbaum-dcp commented Oct 24, 2024

Summary

Adds polling to check for completed subscriber creation, and logs any errors to Papertrail and Sentry.

Tasks/Bug Numbers

Completes #1564

Technical Explanation

Any other info you think would help a reviewer understand this PR?

@dhochbaum-dcp dhochbaum-dcp requested a review from a team as a code owner October 24, 2024 19:30
Copy link

netlify bot commented Oct 24, 2024

Deploy Preview for labs-zap ready!

Name Link
🔨 Latest commit 5ce38b8
🔍 Latest deploy log https://app.netlify.com/sites/labs-zap/deploys/67226e8f721e2d00089da1cc
😎 Deploy Preview https://deploy-preview-1565--labs-zap.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@TylerMatteo TylerMatteo left a comment

Choose a reason for hiding this comment

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

In addition to the comments, just take a look at removing the dependency on @sentry/browser as discussed

server/src/subscriber/subscriber.controller.ts Outdated Show resolved Hide resolved
server/src/subscriber/subscriber.service.ts Outdated Show resolved Hide resolved
@dhochbaum-dcp
Copy link
Collaborator Author

@TylerMatteo Despite much effort, I have not been able to resolve "This branch cannot be rebased due to conflicts". I don't think there's actually an issue. Otherwise, this is ready for review.

@TylerMatteo
Copy link
Contributor

@TylerMatteo Despite much effort, I have not been able to resolve "This branch cannot be rebased due to conflicts". I don't think there's actually an issue. Otherwise, this is ready for review.

It likely has something to do with the merge commit merging develop back onto this branch. I'll enable squash merging for now and, assuming it lets us do that, we can do that for this PR. I was going to suggest squashing at least some of these commits anyway. Long term, we should sit down and figure out how to do rebasing instead of merge commits with Git Tower because this problem will continue to come up.

@dhochbaum-dcp dhochbaum-dcp merged commit 9de1f03 into develop Oct 31, 2024
5 checks passed
@dhochbaum-dcp
Copy link
Collaborator Author

@TylerMatteo Despite much effort, I have not been able to resolve "This branch cannot be rebased due to conflicts". I don't think there's actually an issue. Otherwise, this is ready for review.

It likely has something to do with the merge commit merging develop back onto this branch. I'll enable squash merging for now and, assuming it lets us do that, we can do that for this PR. I was going to suggest squashing at least some of these commits anyway. Long term, we should sit down and figure out how to do rebasing instead of merge commits with Git Tower because this problem will continue to come up.

I did rebase in Git Tower - no idea how it turned out like that.

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