-
Couldn't load subscription status.
- Fork 1.8k
Expose stats ID for use in interceptor factories #3249
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3249 +/- ##
==========================================
+ Coverage 84.01% 84.03% +0.02%
==========================================
Files 80 80
Lines 9041 9041
==========================================
+ Hits 7596 7598 +2
+ Misses 1024 1023 -1
+ Partials 421 420 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| func (pc *PeerConnection) getStatsID() string { | ||
| func (pc *PeerConnection) ID() string { |
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.
@JoeTurki just checking, does this method need to exist in peerconnection_js.go in order for consumers compiling to wasm to be able to access this method (and do we care)? I suspect the wasm build tests are passing because this is a new method, but checking whether it's ok to introduce a new public method here but not in the js/wasm equivalent file.
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.
Good question, we currently don't support stats API in wasm, it would be cool to provide a fallback for stats and pion's interceptor but it will be a lot of work, and we can't support everything.
Thank you.
|
@mengelbart sorry for the ping, is this ready for merge? |
e11b089 to
49a4074
Compare
|
I think it is. |
Description
We are already using the ID to access interceptors. If we expose it, we can store data per peerconnection in interceptor factories and give users an API to interceptors of a specific peerconnection.