-
Notifications
You must be signed in to change notification settings - Fork 55
feat: Ingest API #1469
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
base: release53
Are you sure you want to change the base?
feat: Ingest API #1469
Conversation
Lists internal playlistId and also lists all Rundowns with their externalIds
Moves example to the referenced item for better safety when updating in the future
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
Considering that this is intended to be a part of the REST API, I think it would make sense to use |
meteor/.meteor/packages
Outdated
[email protected] # Meteor's client-side reactive programming library | ||
|
||
zodern:types | ||
fetch |
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.
I don't think we need this in release53
?
meteor/__mocks__/_setupMocks.ts
Outdated
|
||
// Add references to all "meteor" mocks below, so that jest resolves the imports properly. | ||
|
||
jest.mock('meteor/fetch', () => null, { virtual: true }) |
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.
Using regular global fetch
, I think this would be a different mock?
meteor/server/api/ingest/actions.ts
Outdated
logger.info(`Reload rundown: resync request sent to "${resyncUrl}"`) | ||
}) | ||
.catch((error) => { | ||
if (error.errno === 'ECONNREFUSED') { |
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.
Should this handle ENOTFOUND
as well?
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.
That has been added. The idea here is to nicely log most common error messages.
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.
I'm not sure whether adding many of these properties here is a good idea.
As far as sofie is concerned these properties don't do anything, so why should they be at this level instead of inside of the payload?
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.
The idea here is to move all properties that have a direct 1:1 mapping with database fields to the top level of the object, and keep only blueprint-specific or custom logic properties inside the payload object.
For example:
- The
float
andautoNext
properties are fields of the Part model and should be at the top level (just like already existingname
orrank
properties). - In contrast, a property like
partType
, which may be used by blueprints to generate pieces but isn't part of the database schema, belongs inside the payload object.
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 might be best to have a workshop to discuss this. @nytamin can you arrange that?
My concern is that these properties don't do anything unless the blueprints decide that they do. So it feels weird to tell a gateway (with the types) that these properties exist, when it is likely that they will be ignored, and at the same time to tell blueprints that these properties exist when for most ingest sources they won't ever be set.
Presumably these values are added as they are the ones you want, so I am worried that if we start this then more will be added over time morphing this interface into a simplified version of the segment/part we store in the db, with an unclear definition of what can be expected to be set when working in blueprints.
Potentially encouraging the parsing of the actual nrcs data to be done in gateways instead of blueprints (leaving blueprints to re-do a bunch of the parsing and fixup any bad assumptions
The name/rank properties are different in that they are used by sofie-core for something (even if that is only logging like name is)
I should probably add this part somewhere else, but it is relevant to this discussion. (Sorry, this didnt occur to me until I was looking into this again just now)
In the MutableIngestSegment
and related types, these properties become required to have a value (by providing defaults if they are not defined), which feels wrong in another way, as now we have lost that the nrcs/api did not provide these values. If they stay, I think that preserving them being undefined is important.
Additionally, MutableIngestSegment
has getters for these properties but not setters, and they are not updated by defaultApplyIngestChanges
meaning that changes to these values for existing parts/segments/rundowns will not be propogated through to blueprints.
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.
Due to vacations, this will be idle from EVS' side for a couple more weeks before we come back to resolving this pending decision.
|
||
getSegment(id: SegmentId): PlayoutSegmentModel | undefined { | ||
return this.segments.find((segment) => segment.segment._id === id) | ||
return this.segments.find( |
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.
I don't think it is the responsibility of the playout model to handle the api using ambiguous ids.
Instead you could add a getSegmentByExternalId
to make this lookup explicit
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.
Moved to the API layer.
|
||
getPart(id: PartId): ReadonlyDeep<DBPart> | undefined { | ||
return this.parts.find((part) => part._id === id) | ||
return this.parts.find((part) => part._id === id || part.externalId === unprotectString(id)) |
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.
same problem here as with getSegment
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.
Moved to the API layer.
meteor/server/security/check.ts
Outdated
}, | ||
})) as Pick<DBRundownPlaylist, '_id' | 'studioId' | 'organizationId' | 'name'> | undefined | ||
const playlist = (await RundownPlaylists.findOneAsync( | ||
{ $or: [{ _id: playlistId }, { externalId: playlistId }] }, |
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.
I think it would be best to avoid having these fuzzy checks out of core logic, and keep it inside of the api implementation.
There are a few places where this is done in this PR. It might require a separate/explicit query to convert an externalId
to _id
, but this should be safe to do as it is a stable function for converting an externalId to an _id
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 has been moved to API layer.
About the Contributor
This PR is posted on behalf of EVS Broadcast Equipment.
Type of Contribution
This is a: Feature
State of the work offered
The feature - What, why and how
An HTTP REST API for creating and updating rundown data.
This allow standard web clients to connect to Sofie to serve rundowns and rundown content, taking on the role that traditional NRCSes have in existing workflows. Sofie is already well covered on traditional workflows with the MOS workflows. This addition of HTTP REST API represents a step in the direction of open and web-centric systems which will further enable integrations and smooth workflows.
The implementation follows established patterns and is implemented in two separate parts: an OpenAPI definition and one part in the existing REST API implementation. A summary of the changes is listed at the end of this document.
Example use cases
Example 1: HTTP-based NRCS/CMS
*not comparable to the full blown DDP-based PeripheralDevice gateways.
Example 2: Headless Sofie
Topologies
Design, considerations and choices made
RESTful principles are followed
The verbs GET, POST, PUT, DELETE are used.
Each of Playlists, Rundowns, Segments and Parts are made available as resources, following this pattern:
/ingest/{studioId}/playlists/{playlistId}/rundowns/{rundownId}/segments/{segmentId}/parts/{partId}
where, for example,/ingest/{studioId}/playlists
is a collection and/ingest/{studioId}/playlists/{playlistId}
is a resource in the Playlists collection.Calling
DELETE
on a resource will always also remove any resources owned by the deleted resource. Calling PUT on a collection will replace all resources of the type under that collection - i.e. when an array of e.g. Segments is received for a Rundown, if there are any already existing Segments within the Rundown that are not included in the received array then those Segments will be deleted. In this way, the entire contents of a Rundown can be replaced by sending a new array of Segments via a singlePUT
.The
playlistId
,rundownId
,segmentId
, andpartId
can be any unique ID of the external system's choosing, so long as they are consistent for a particular resource.20x Response codes
20x OK means the request itself is valid and the system goes off doing the asynchronous ingest operation - which can later fail, with no way of communicating this back to the external system.
There is an opt-in mechanism for Blueprints validation of payloads during processing of the request. If specified, a Blueprint function can do synchronous validation before Core responds on the request. A failed validation in this Blueprints-hook results in a 40x error.
Even after a successful validation by Blueprints, the Core ingest job will perform asynchronously and might fail in silence after having reported 20x back to the external client.
Playlists lifespans in relation to Rundowns
playlistExternalId
are merged into one Playlist, and Rundowns with uniqueplaylistExternalId
s live in discrete Playlists.GET
andDELETE
methods for Playlists, but noPOST
orPUT
.The Resync mechanism
MOS allows Sofie to request an update of all rundown data. This user-facing feature is intended to allow users to refresh the rundown data if they observe something is outdated or wrong.
The same mechanism is implemented for this HTTP Ingest API:
ExternalIds
With exception of the
studioId
. the external system creates and owns all IDs in the/ingest/{studioId}/playlists/{playlistId}/rundowns/{rundownId}/segments/{segmentId}/parts/{partId}
structure. These IDs map to externalId in the Sofie database.For the external system to be able to control Sofie (Use case 2, "Headless Sofie"), it needs to be able to do rundown playout operations in an efficient way. Currently, the User Actions API expects internalIds for its operations. This could have meant that the external system would need to query back all content it has created, in order to create and store a map of the relation between external and internal IDs in memory, to be able to send any playout command, which wouldn't be pleasant to work with as a consumer of the APIs.
For this reason, we have simplified this by allowing either internalId or externalId to be used interchangeably on the User Actions API. This greatly simplifies/enables the described Use case 2, "Headless Sofie".
Rundown, Segment, and Part models
Rundowns, Segments, and Parts are modelled to match common public properties and database structure.
The Part payload property is un-typed, and will match the Blueprint's expectation to incoming content needed to create Parts with Pieces.
The structure allows for a closed loop where Blueprints define the schemas, which external systems use to generate the requests with typed payload, which ultimately gets synchronously parsed and validated during ingest by Blueprints again.
Security and auth is excluded from the scope of this PR
Following existing patterns/not unique for this Ingest API addition.To be dealt with elsewhere in the stack or in a more global matter.
Summary of changes
Design: packages/openapi/
Implementation: meteor/server/api/rest/v1/ingest.ts
./index.ts
through the establishedregister*Router(sofieAPIRequest)
.runIngestOperation()
) in the same way as the existing ingestMosDevice.httpIngest
(rundownSource type httpIngest), which can be confusing considering the existing non-REST "http ingest api" and rundownSource type "http".packages/blueprints-integration/src/api/studio.ts
Side-effect: packages/shared-lib/src/peripheralDevice/ingest.ts
Side-effect: Job worker playout implementation
See the paragraph on "ExternalIds"
Changes to lookup of playlists, rundowns, segments and parts in the following places:
Testing
Status