Audit Engine API (Architecture and API)

changed milestone to %1.0-alpha2 — Precise Pheasant

added P3 label

I think everything is pretty much done here. The only thought I had if we should include some "studio id" or "studio name" for live entries. This would allow the studio clock to display where the live show is actually happening.

Thoughts? Missing something else in the playlog entries?

changed the description

added To Do label

added Doing label and removed To Do label

Yep, the studio name would be nice. Not sure where to have it configured. Depends on how the line:// entries work? This should be a radio wide setting, because tank accepts any text after the line:// part, and dashboard just uses a preconfigured set of line ins. Ideally this could be named line ins, like line://studio1 or line://live_studio, then this info could be used in the clock without further need of customisation?

added 1 deleted label

Thanks for the thoughts. Storing these source IDs for each playlog makes absolutely sense. In Engine the channels are hardcoded strings, mapped to the IDs you have mentioned:

FILESYSTEM_A = "in_filesystem_0"
FILESYSTEM_B = "in_filesystem_1"
HTTP_A  = "in_http_0"
HTTP_B  = "in_http_1"
HTTPS_A  = "in_https_0"
HTTPS_B  = "in_https_1"
LIVE_0 = "linein_0"
LIVE_1 = "linein_1"
LIVE_2 = "linein_2"
LIVE_3 = "linein_3"
LIVE_4 = "linein_4"

The could be stored for each playlog entry in the API. And in the Studio Clock there could be a configurable mapping to a prettified string like "linein_0" resolves to "Live Studio A".

The only thing which bothers me, it is stored for each and every playlog entry. But we could only store the ones for live sources, and skip file and stream sources. On the other hand this info might be helpful for audits and debugging purpose (a tiny bit).

@sarah.kanawin

Addressing technical people of Radios, please check and verify the provided fields in the API: https://app.swaggerhub.com/apis/AURA-Engine/engine-api/1.0.0

Is there some information missing for reports or the track service? Anything you might need in the future?

added 1 deleted label

Hey, I finally got around to do my "audit" of README.md and the API spec. Here my results, I hope they are comprehensible (if not, please ask, we can also do a quick call on that):

AURA Engine API doc and spec audit notes

In the repo's README.md

Overview section:

"Track Service: Same as track service, but stripped-down information. Used for implementing a track service feature on the radio's website."

Should this be "Same as playlog"?

Deployment Modes section:

"Next, it also performs a request to the Synchronization API Serve."

Here an "r" seems to be missing at the end.

The section on playlog syncing might be a bit hard to get. Or just missing an explanation why the sync server is doing the batch syncing after an outage, while the "not synced" status is written in local db of the engine servers. I mean, it does make sense after a bit of thinking, but I thought this is a bit hard to get at first. Not sure if we should explain this better, or if this is just a given, people have to take.

Ok, I'll try to make it more clear what I mean, on the basis of a network outage example. The engine certainly realizes that there is a network outage, because it cannot sync the info of its newly played audio file. So it just marks it as unsynced and don't bothers any more. Fine. But how does the sync server know there is/was a network outage and that it has to check when the network is back up to start the batch syncing job? Wouldn't it be more reasonable that the engine server itself starts the batch job as soon as it realises it can sync again?

Getting Started section:

I didn't try it out atm. but it looks very well described, with all the infos I would want to have when setting it up.

Running the Server section:

Also very well described. I only found some minor things to improve:

"Next login to engineuser and give it permissions to the unit file"

Here I would either leave out the su engineuser in the following instructions, as all other commands are run as sudo anyways, or - if it is a prerequisite - explain why we log in first as engineuser. Because in a common scenario it does not make sense, because the commands run as sudo would anyway run with root privileges, and the user for the systemd service is defined in the systemd config file itself.

And the tail -f /var/log/syslog might need a preceding sudo, as on many systems the syslog has no read permissions for standard users (even if they are in the sudo group)

Development section:

Also everything there I need to jump in.

That is of course, if I am familiar with OpenAPI, Flask, Connexion and Docker. But I think we don't have to mention that explicitly, because I trust developers will familiarize themselves with whatever frameworks they will be developing with. But it might be nice to provide links to good starting points for people who just stumbled by and would like to play around with the engine API, yet do not have experience with those things.

The OpenAPI definition

Endpoints:

The GET /trackservice endpoint could also need a from_date and to_date filter. Not sure if we really need it, but it might come in handy at some point? E.g. to query all tracks played on a specific day.

What does the PUT /clock exactly do? Shouldn't the studio clock info be a pull info from the engine, generated out of the pushed playlogs? Who is supposed to put a separate studio clock info?

The examples for from_date and to_date as query parameters for the playlog entry are rendered as "Ordered Map {}". Maybe put quotes around the example string. So instead of example: 2020-11-29T09:12:33.001Z do example: "2020-11-29T09:12:33.001Z". Also in the schema you can add a format: date-time additionally to the type: string constraint (see OpenAPI spec on Data Types)

Schemas

What does the track_type in a Track, PlayLog or PlaylistEntry mean? I think the other properties are more self-evident, but this one might need a description: .

Track vs PlayLog: do we need to separte schemas if they are basically the same, only with an additional log_source in the PlayLog?

ClockInfo: here we have a engine_source property as well a log_source inside the current_track PlayLog. Can they be different? Does it make sense to have this redundancy? And regarding the current and next schedule, see next comment.

Schedule: Do we really mean an AURA Schedule or rather a Timeslot with that? It looks like a timeslot to me, and then I would name it accordingly. Maybe Timeslot was not the best label for what it means, but I'd now stick with it and be project wide consistent. Also because it is confusing what the schedule_id property of the Schedule then means.

General conclusion

This already looks all quite well and thought through. I could not come up with anything else we would urgently need from the API. But maybe others still have other use cases. The few things I found might mostly be of cursory nature and not functionally relevant. But by resolving them we could improve the already high approachability of this component even more.

mentioned in issue #12 (closed)

@jackie WOW!! Thank you so much. I didn't expect such a thorough review, but it makes total sense.

Should this be "Same as playlog"?

fixed.

Here an "r" seems to be missing at the end.

fixed.

The section on playlog syncing might be a bit hard to get.

Improved and extended the explanation here. Please check - Do you think it's enough or doesn't it need even more fine-grained explanations?

Here I would either leave out the su engineuser in the following instructions,

removed...

But it might be nice to provide links to good starting points for people who just stumbled by and would like to play around with the engine API, yet do not have experience with those things.

Added links to these dependency projects.

The GET /trackservice endpoint could also need a from_date and to_date filter.

Created ticket: #12 (closed)

What does the PUT /clock exactly do? Shouldn't the studio clock info be a pull info from the engine, generated out of the pushed playlogs? Who is supposed to put a separate studio clock info?

There is no functionality where anything can be pulled from engine. Engine itself doesn't have a Web API. It's only the "Engine API" project which has API endpoints. Any data from Engine itself is PUT or POSTed to its local Engine API instance.

No, the studio clock cannot generate the studio clock data purely out of the submitted playlogs either. The studio clock info consists of current_track (== current playlog), current_playlist (here it's discussable if the past playlog entries should be calculated based on past playlogs of the current schedule, as you suggested; this surely doesn't work for future entries), current_schedule and next_schedule (This information is PUT by engine-core to it's local engine API instance whenever the current playing track changes. It is furthermore also PUT to the engine-api sync node.

@jackie let's #DISCUSS the part with past playlist entries at least. And if we have a similar understanding of "pulling things from engine".

The examples for from_date and to_date as query parameters for the playlog entry are rendered as "Ordered Map {}". Maybe put quotes around the example string.

Thanks. Added quotes and format: date-time. Created task to merge updated API to the codebase: #13 (closed)

What does the track_type in a Track, PlayLog or PlaylistEntry mean?

0 .. Filesystem Entry 1 .. Stream Entry 2 .. Live / Line In Entry

Added those as description in the API Spec.

Track vs PlayLog: do we need to separte schemas if they are basically the same, only with an additional log_source in the PlayLog?

Let's #DISCUSS this too. It also depends on wheter we want to add more internal props for debugging and auditing, such as:

log_time ... when the playlog was written to the local API database
expected_play_time ... when the playlog was acutally scheduled for playout
something like studio_id or line_in_id ... to identify from which studio a live entry is coming from

And if such props need to be kept "secret".

ClockInfo: here we have a engine_source property as well a log_source inside the current_track PlayLog. Can they be different? Does it make sense to have this redundancy? And regarding the current and next schedule, see next comment.

Technically it could be possible, but hardly. At the moment only in some weird fallback scenario where Engine 1 or 2 are switching in the very moment between some playlog logging is happening on Engine 1 and clock info logging is happening on Engine 2.

One (future) scenario to make use of this seemingly redundancy, might be: The current scheduled show is logged from e.g. Engine 1, but the currently playing track is not part of that show's planned playlist, but logged from a Silence Detector. This would be the case if we integrate a 3rd party Silence Detector in a way that it also reports playlogs / track service info.

For better understanding, why this is duplicated already:

EngineClock data PUT is using this field to check if it's coming from an active engine. Same approach as with playlogs and its sync.
The EngineClock field current_track is not actually stored in the clock info in the database. this information is dynamically read from the playlog database, as soon it is requested. Hence the field is already in the database and in the playlog schema it is duplicated.

Schedule: Do we really mean an AURA Schedule or rather a Timeslot with that? It looks like a timeslot to me, and then I would name it accordingly. Maybe Timeslot was not the best label for what it means, but I'd now stick with it and be project wide consistent. Also because it is confusing what the schedule_id property of the Schedule then means.

I think this one needs some discussion and review. Within Engine there is no mention of the term "timeslot" nowhere, since the beginning. As far as I see also all fields from Steering API seem to be labeled schedule_id or schedule_fallback_id or similar. So I tried to stay consistent in Steering and Engine context. But maybe I'm missing something - in that case I will update. Let's review this one together. #DISCUSS

This already looks all quite well and thought through. I could not come up with anything else we would urgently need from the API. But maybe others still have other use cases. The few things I found might mostly be of cursory nature and not functionally relevant. But by resolving them we could improve the already high approachability of this component even more.

A thousand thanks for you deep and thoughful review! =)

changed the description

mentioned in commit 37f7922a

added Blocked label and removed Doing label

assigned to @david

mentioned in commit 9799652a