-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
change signature of orphanedThreadFilter #45469
change signature of orphanedThreadFilter #45469
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @stephane-airbyte and the rest of your teammates on Graphite |
0e70ed1
to
6330d30
Compare
06e8d2c
to
2e5fdf1
Compare
6330d30
to
3e0f516
Compare
2e5fdf1
to
dd77b79
Compare
3e0f516
to
1ded4bf
Compare
9ea7da7
to
b9fbff4
Compare
7af5625
to
b5ed686
Compare
9d78f10
to
daab75f
Compare
b5ed686
to
72151ff
Compare
daab75f
to
708b5b8
Compare
72151ff
to
162f055
Compare
708b5b8
to
fee0563
Compare
48fb5f0
to
59b16fb
Compare
fffb246
to
90e3ffd
Compare
f49e6a6
to
90c1a66
Compare
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.
changes seem reasonable, though I'm unclear on what the thread info actual race condition is - can you say a bit more about that?
(everything else looks like nice qol improvements + the sad stderr thing)
@@ -493,6 +517,7 @@ internal constructor( | |||
} | |||
|
|||
private fun dumpThread(thread: Thread): String { | |||
OrphanedThreadInfo.getForThread(thread) |
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.
this doesn't seem to do anything?
// stdout while we run this test, in which case we'd see their messages. | ||
// we filter through the messages to find an error (hopefully hours) | ||
for (line in outCt.split('\n')) { | ||
if (line.contains("\"error\"")) { |
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.
😢
it looks like we're expecting a json blob - can we filter for something like "type":"trace"
? or maybe filter for lines that are valid json
(otherwise wouldn't this trigger for any logger.warn("error while blah blah")
message?)
90c1a66
to
c35540c
Compare
OrphanedThreadInfo
data class to encapsulate thread information to avoid data races when getting thread-specific information (like stacktrace, or threadLocal values)OrphanedThreadInfo
LoggingInvocationInterceptor
AirbyteTraceMessageUtilityTest
TestContext
to use a default value forCURRENT_TEST_NAME