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

chore: add log when we fail to insert a stream into a model table #3222

Closed
wants to merge 1 commit into from

Conversation

stephhuynh18
Copy link
Contributor

Description

Include a summary of the change or link to the issue it addresses.

Include relevant motivation, context, brief description and impact of the change(s). List follow-up tasks here.

How Has This Been Tested?

Describe the tests that you ran to verify your changes. Provide instructions for reproduction.

  • Test A (e.g. Test A - New test that ... ran in local, docker, and dev unstable.)
  • Test B

PR checklist

Before submitting this PR, please make sure:

  • I have tagged the relevant reviewers and interested parties
  • I have updated the READMEs of affected packages
  • I have made corresponding changes to the documentation

References:

Please list relevant documentation (e.g. tech specs, articles, related work etc.) relevant to this change, and note if the documentation has been updated.

.merge(toMerge)
.catch((err) => {
this.logger.err(
`Error indexing ${JSON.stringify(
Copy link
Contributor

Choose a reason for hiding this comment

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

if you're going to log json data, I suggest putting it at the very end of the log message, otherwise the message can get hard to read. So I'd suggest rewording slightly so that model name and the error message comes first, with the arbitrary json data at the end.

@stephhuynh18 stephhuynh18 deleted the add-error-logging-failed-mid-insert branch July 22, 2024 15:44
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