Skip to content
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

Proposal for a way to handle multiple concurrent activations #15

Merged
merged 6 commits into from
Dec 20, 2018

Conversation

simonrankine
Copy link
Contributor

No description provided.

@garethsb
Copy link
Contributor

How are activation ids allocated? (By the server)
Are they returned in the response to the POST? (Location header?)
Will activation ids be reused?

Would this mechanism support/be extendable to support queued activations in the future?

@chris-gil
Copy link
Contributor

The example response to /map/active is in examples/map/map-tables-get-200.json
This file begins with an activation object:

{
  "activation": {
    "mode": null,
    "requested_time": null,
    "activation_time": null
  },

Should this still be included or has all the information about pending activations moved to /map/activations?

@simonrankine
Copy link
Contributor Author

/activations is currently information about activations that are going to happen, whereas that would indicate information about the previous activation.

@simonrankine
Copy link
Contributor Author

How are activation ids allocated? (By the server)
Are they returned in the response to the POST? (Location header?)
Will activation ids be reused?

Would this mechanism support/be extendable to support queued activations in the future?

  • Doesn't matter how they are allocated as long as they are unique, but yes it's up to the server to do so
  • Yes, they are returned in the body of the response to the POST
  • Do you mean in time, or across API instances? Ideally not chronologically.

What do you mean by "queued" in this context? It already supports scheduling activations into the future, is that what you mean?

@garethsb
Copy link
Contributor

  • Doesn't matter how they are allocated as long as they are unique, but yes it's up to the server to do so
  • Do you mean in time, or across API instances? Ideally not chronologically.

Yes, I meant chronologically. Would the idiomatic NMOS approach to this be UUIDs or (uniqued) TAI timestamps? (The usual syntax for the latter is currently ruled out by the regex.) What do we lose by mandating a particular approach (as we do for e.g. Query API subscriptions)?

  • Yes, they are returned in the body of the response to the POST

Ah. It seems a bit odd to me that the response to a POST to create a single activation responds with all the future activations.

What do you mean by "queued" in this context? It already supports scheduling activations into the future, is that what you mean?

I meant multiple changes to the same input or output. I guess it does?

If two activations for the same input/output at the same time are requested, is the second one rejected (e.g. 423)?

@simonrankine
Copy link
Contributor Author

  • Doesn't matter how they are allocated as long as they are unique, but yes it's up to the server to do so
  • Do you mean in time, or across API instances? Ideally not chronologically.

Yes, I meant chronologically. Would the idiomatic NMOS approach to this be UUIDs or (uniqued) TAI timestamps? (The usual syntax for the latter is currently ruled out by the regex.) What do we lose by mandating a particular approach (as we do for e.g. Query API subscriptions)?

The reason I've steered away from using UUIDs is that within NMOS they are usually used to represent system wide identifiers, rather than identifiers within a single API - as would be the case here. Something like hashing the system time would suffice.

  • Yes, they are returned in the body of the response to the POST

Ah. It seems a bit odd to me that the response to a POST to create a single activation responds with all the future activations.

I thought what I'd done was that the POST returned with an object with only one entry, which is indexed by the activation ID and contained the activation object created, but looking at the schemas that doesn't appear to be the case. That's the intention, I'll try update the docs so they reflect that!

What do you mean by "queued" in this context? It already supports scheduling activations into the future, is that what you mean?

I meant multiple changes to the same input or output. I guess it does?

If two activations for the same input/output at the same time are requested, is the second one rejected (e.g. 423)?

Ah I see. My intention was that if an Output was already implicated in a staged activation it would be locked, and any attempt to involve it in another activation would cause it to be rejected with a 423. Or things get complicated very quickly...

@garethsb
Copy link
Contributor

The reason I've steered away from using UUIDs is that within NMOS they are usually used to represent system wide identifiers, rather than identifiers within a single API - as would be the case here. Something like hashing the system time would suffice.

Personally, I don't like the inventiveness here. A (e.g. random or time-based) UUID would solve the issue in a consistent manner. (Query API WebSocket subscription is also API-local.)

the POST returned with an object with only one entry, which is indexed by the activation ID and contained the activation object created. I'll try update the docs so they reflect that!

Thanks!

Ah I see. My intention was that if an Output was already implicated in a staged activation it would be locked, and any attempt to involve it in another activation would cause it to be rejected with a 423. Or things get complicated very quickly...

That certainly makes sense.

So each output can appear in at most one activation. With IS-05, because there is a one-to-one relationship between Sender/Receiver and the activation object, when you get a 423 Locked, it's clear what you need to do. That's a bit harder for the client here?

@maxwalter79
Copy link

So each output can appear in at most one activation. With IS-05, because there is a one-to-one relationship between Sender/Receiver and the activation object, when you get a 423 Locked, it's clear what you need to do. That's a bit harder for the client here?

I'm afraid its not the case, because the activation is now (in current proposal) on "per channel" basis. So there could be many pending (staged) activations for a single "output".

I would also prefer the idea to have just 1 staged activation on "per output" basis.
I don't think it should be this granular and complex at all. But maybe I'm wrong here.

@garethsb
Copy link
Contributor

So each output can appear in at most one activation.

I think I meant channel:

So each channel can appear in at most one activation.

But this still applies:

With IS-05, because there is a one-to-one relationship between Sender/Receiver and the activation object, when you get a 423 Locked, it's clear what you need to do. That's a bit harder for the client here?

I.e. you have to find which activation includes that channel.

@chris-gil
Copy link
Contributor

/activations is currently information about activations that are going to happen, whereas that would indicate information about the previous activation.

Would that be the most recent activation?

@billt-hlit
Copy link
Contributor

The post response needs to contain the activation ID so that the client isn't forced to hunt down the activation ID in order to delete it. The example, at least, doesn't show such an ID.

Copy link

@jasperpeeters jasperpeeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response of GET on /map should be updated to

[
  "activations",
  "active"
]

according these changes.

Copy link

@jasperpeeters jasperpeeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The schema + example for GET /map/active does still contain one single activation object on the root level.
Not sure how to reflect the activation exactly on this endpoint, but I think that needs to be updated as well.

@jasperpeeters
Copy link

One question about your newest update in 8a03df4 @simonrankine :

Here you added in the Activation Requests section:

Changes to the activations resource MUST NOT result in any changes to the Device audio channel mapping behaviour until there is an activation.

and futher:

In order to trigger an activation a client POSTs an activation object in the activations endpoint. [...]

For me this reads like it would be possible to split an audio mapping change request into 2 separate POST requests, one containing only the action json object and one only containing the activation json object.
After POSTing the first one, how would you target / reference the previously posted action with a POST only containing the activationobject?

I noticed that you mentioned in the very first comments on this PR that an activation_id is returned with the POST response. (That's missing in the schemas + examples as @billt-hlit already mentioned.)
If that should be used, it would possibly require an additioinal PATCH or POST method on /map/activations/{activation_id}

@jasperpeeters
Copy link

In the section Activation Responses is written, that you should return (in short):

  • 200 for successful immediate activation
  • 202 for accepted scheduled activation
  • 423 if there is already a scheduled activation (SHALL reject entire request!)
  • 400 if routing operation not permitted (SHALL reject entire request!)

First this is a bit misleading, as if the request is technically OK, it should return a 200 to the POST request itself.
But I assume the mentioned codes relate to the status key in the POST response example:

{
  "activation":{
    "mode": "activate_scheduled_absolute",
    "requested_time": "1544448739:0",
    "activation_time": "1544448739:0"
  },
  "action":{
    "OutA":{
      "0":{
        "input": "input1",
        "channel_index": 2,
        "status": 200
      },
      "3":{
        "input": "input1",
        "channel_index": 1,
        "status": 423
      }
    }
  }
}

So either the request succeeded and you got the same status_code for each channel (as the same activation_object (in particular it's mode) applies to all mentioned channels), or it failed at least once, where you should reject the entire request.

Is a per-channel indication with a status really needed?

Related minor things which I noticed reading the changes:

  • the example has mixed status (200 and 423) in it's response, although this is not recommended by the docs (-> SHOULD reject entire request)
  • the status key is not mentioned the schema

@simonrankine
Copy link
Contributor Author

Okay, I've just pushed up changes that I hope mop up all the comments above.

I'm not sure how the status entries in the POST response crept in, I think it was an idea I was playing around with at some stage that ended up where it shouldn't have. Apologies for any confusion. The idea is that either the entire activation is acceptable, or the entire activation is rejected, and that state is reflected in the HTTP response code in reply to the POST.

I've also added the ID into the post response, which was always meant to be there but got lost in a bit of copy-paste editing.

I've tidied up the language around the POST requests that jasper queried.

Changes to the activations resource MUST NOT result in any changes to the Device audio channel mapping behaviour until there is an activation.

This was only ever meant to convey that pending activations shouldn't make any changes until the actual activation time. Obvious if you've done NMOS stuff before, but an important point to make as I've seen this mistake made with IS-05 staged parameters before. The POSTs are all or nothing, the activation object and corresponding map are both in the request together.

In order to trigger an activation a client POSTs an activation object in the activations endpoint.

I can see how this added to the confusion, there's a implicit "and map object" there which I've now added explicitly to the text.

I think this clears up all the comments in this thread, but if not give me a shout.

Copy link
Contributor

@chris-gil chris-gil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good to me.
thanks,
Chris

@simonrankine simonrankine merged commit c5bde50 into v1.0-dev Dec 20, 2018
@peterbrightwell peterbrightwell deleted the simonra-multi-activation branch May 22, 2020 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants