-
Notifications
You must be signed in to change notification settings - Fork 50
Conversation
@AetherUnbound you said that in real usage these notifications would likely be sent hours apart right? Just wanting to confirm that, otherwise I might suggest somehow updating the existing message for successful passes or something instead of sending a new one (but still sending a new message if it fails) 😅 Should failures ping |
Yes! In production, it'll be a few hours between each step. For example, here are the rough runtimes based on the
As for failures, I don't think we need to |
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 👍
# The server sometimes hangs on or before this next step. Adding this debug | ||
# message here because apparently that unblocks it? May have to do with GC. | ||
log.debug("Getting downstream cursor again") |
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.
Could try verifying this using gc.collect
?
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.
I'm guessing that any additional call made here, prior to the database connection call, would unblock this step. Work a shot though 🤷♀️
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.
Hmmm... any additional call... would that include like int("0")
or something? I wonder what just calling a function could be causing it to clear 🤔
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.
I'm not sure! What I noticed is that all of the following would cause code execution to continue as expected:
log.info
log.debug
print
I'm find leaving in the GC and seeing if it pops up again. It's frustrating because it's so intermittent!
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.
Interesting. I wonder if it's somehow IO related?
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.
Maybe! I'm really at a loss
b7f3629
to
1c8f2db
Compare
Fixes
Fixes #412 by @AetherUnbound
Description
This PR adds a very simple slack notification mechanism for the ingestion server. During data refresh, the ingestion server will give periodic updates in slack regarding the progress of a given data refresh. If the refresh fails at any point, the ingestion server will also send the exception to Slack.
Examples:
Testing Instructions
SLACK_WEBHOOK
variable inenv.docker
just ingest-upstream
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin