Allow to use ManifestID provided by AuthWebhook in HTTP push flow #1627
Conversation
darkdarkdragon
force-pushed
the
it/manifest
branch
from
ec86afa
to
0eaea7e
Compare
|
Presently doing an integration test with this in mdw-staging |
darkdarkdragon
force-pushed
the
it/manifest
branch
2 times, most recently
from
0eaea7e
to
824bb89
Compare
|
Seems to be working well. |
|
Hm I see there was a force push with 824bb89, but doesn't look like anything changed? Was that intended @darkdarkdragon ? |
|
I think that was a rebase onto master to pick up your auth token branch |
| @@ -701,7 +713,7 @@ func (s *LivepeerServer) HandlePush(w http.ResponseWriter, r *http.Request) { | |||
|
|
|||
| // Start a watchdog to remove session after a period of inactivity | |||
| ticker := time.NewTicker(httpPushTimeout) | |||
| go func(s *LivepeerServer, mid core.ManifestID) { | |||
| go func(s *LivepeerServer, intmid, extmid core.ManifestID) { | |||
There was a problem hiding this comment.
Only need to pass in extmid here since removeRTMPStream() just needs the external manifestID now.
There was a problem hiding this comment.
fixed in 63d4ab6
Need both arguments because we also need to index rtmpConnections inside watchdog function.
There was a problem hiding this comment.
Oh I see...Got it. Perhaps there is room for further hiding the notion of an internal manifestID in this case so a user of rtmpConnections only ever has to worry about the external manifestID...think that's fine to address separately.
| @@ -701,7 +713,7 @@ func (s *LivepeerServer) HandlePush(w http.ResponseWriter, r *http.Request) { | |||
|
|
|||
| // Start a watchdog to remove session after a period of inactivity | |||
| ticker := time.NewTicker(httpPushTimeout) | |||
| go func(s *LivepeerServer, mid core.ManifestID) { | |||
| go func(s *LivepeerServer, intmid, extmid core.ManifestID) { | |||
There was a problem hiding this comment.
Oh I see...Got it. Perhaps there is room for further hiding the notion of an internal manifestID in this case so a user of rtmpConnections only ever has to worry about the external manifestID...think that's fine to address separately.
darkdarkdragon
force-pushed
the
it/manifest
branch
from
63d4ab6
to
776fac3
Compare
What does this pull request do? Explain your changes. (required)
Allow to use ManifestID provided by AuthWebhook in HTTP push flow
Specific updates (required)
Added map that keeps mapping between ManifestID sent in HTTP push URL and one provided by webhook
How did you test each of these updates (required)
Added unit tests
Does this pull request close any open issues?
Fixes #1528