Skip to content

Conversation

@ghukill
Copy link
Contributor

@ghukill ghukill commented Aug 12, 2024

Purpose and background context

When performing upserts to Quickbase, even if some records have errors, the HTTP response will be 200. It is required to parse the response and see what specific records had errors. We capture this grain in a method to aggregate upsert results across all tasks, but we only logged it.

Now that all upsert errors have been resolved as a baseline, anything new indicates new data or an edge case we have not encountered. As such, it is a good time to introduce more warning when this happens.

How this addresses that need:

  • Manually send a Sentry message when upsert errors detected

Includes new or updated dependencies?

NO

Changes expectations for external applications?

YES: Sentry messages sent

What are the relevant tickets?

Developer

  • All new ENV is documented in README
  • All new ENV has been added to staging and production environments
  • All related Jira tickets are linked in commit message(s)
  • Stakeholder approval has been confirmed (or is not needed)

Code Reviewer(s)

  • The commit message is clear and follows our guidelines (not just this PR message)
  • There are appropriate tests covering any new functionality
  • The provided documentation is sufficient for understanding any new functionality introduced
  • Any manual tests have been performed or provided examples verified
  • New dependencies are appropriate or there were no changes

Why these changes are being introduced:

When performing upserts to Quickbase, even if some records have
errors, the HTTP response will be 200.  It is required to parse
the response and see what specific records had errors.  We capture
this grain in a method to aggregate upsert results across all tasks,
but we only logged it.

Now that all upsert errors have been resolved as a baseline, anything
new indicates new data or an edge case we have not encountered.  As
such, it is a good time to introduce more warning when this happens.

How this addresses that need:
* Manually send a Sentry message when upsert errors detected

Side effects of this change:
* Sentry messages sent

Relevant ticket(s):
* https://mitlibraries.atlassian.net/browse/HRQB-46
Copy link

@ehanson8 ehanson8 left a comment

Choose a reason for hiding this comment

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

Clear logic, great work!

Copy link

@jonavellecuerdo jonavellecuerdo left a comment

Choose a reason for hiding this comment

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

Sounds good to me! ✨ Approved but curious what you mean by "all upsert errors have been resolved as a baseline"?

@ghukill
Copy link
Contributor Author

ghukill commented Aug 15, 2024

Sounds good to me! ✨ Approved but curious what you mean by "all upsert errors have been resolved as a baseline"?

Ah, good question. This is related to this PR, which resolved the last upsert errors. Until then, a given run had a handful of errors across a handful of load tasks.

Now that we've resolved all those, and have a good "baseline" of error-free, the timing was good to setup alerts for new errors that crop up. Otherwise, we would have been getting these Sentry errors daily 😅 .

@ghukill ghukill merged commit 5a35f77 into main Aug 15, 2024
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.

3 participants