-
Notifications
You must be signed in to change notification settings - Fork 12
feat: Add interactive listen and --filter-x param for session filtering #161
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
Conversation
@claude review |
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.
did a light test against staging, confirmed the filter behavior is working as expected
TargetURL *url.URL | ||
Sources []*hookdecksdk.Source | ||
Connections []*hookdecksdk.Connection | ||
Filters interface{} // Session filters (stored as interface{} to avoid circular dependency) |
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.
what's the circular dependency?
if activeCount > warningThreshold && !p.maxConnWarned { | ||
p.maxConnWarned = true | ||
p.renderer.OnConnectionWarning(activeCount, p.transport.MaxConnsPerHost) | ||
} else if activeCount < resetThreshold && p.maxConnWarned { | ||
// Reset warning flag when load decreases | ||
p.maxConnWarned = false | ||
} |
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.
@alexbouchardd My version warned whne approaching max connections https://github.com/hookdeck/hookdeck-cli/pull/153/files?new_files_changed=true&diff=unified#diff-b41d02f71c670ff801d99892d32ef6dd36e43fcdf4c12c044cd34a073cc18200R285-R296 Probably something to keep? This code flips maxConnWarned to true but doesn't seem do anything about it.
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.
p.renderer.OnConnectionWarning(activeCount, p.transport.MaxConnsPerHost)
handles the rendering
No description provided.