Disable checking in encoding profile

[v0tti]

It should be possible to disable checking for certain encoding profiles. On many events the "subformats" are released without checking.

[saerdnaer]

When implementing this issue, I would opt for a generic way to disable different states configurable in the encoding profile – c.f. cloud encoding.

[jjeising]

There is already an unmerged (private) pull request that needs some minor work. Will merge soon.

[jjeising]

Working on this in #162.

[a-tze]

Counter-proposal: make checking an automatable state.

Advantages:

• changes to data model are very small, no structural changes AFAICS
• some automated checking (minimum: proper decoding, check of duration) is better than no checking at all and maybe even better than human checking, as is demonstrated by this issue

Disadvantages:

• Checking worker will usually need to have a ticket filter to not work on master tickets. Forgetting this filter will be a likely trapdoor

[MaZderMind]

@a-tze can't a checker-worker filter on the encoding-profile-properties? In the early days we had room .filter as part of the API – i don't know if that part of the API is still there.

[a-tze]

@MaZderMind Yes, having a filter in the worker is a valid counter-measure, but I did not want that to be a mandatory part of the concept ;)

That part of the API is still there, because there is no other way for the recording worker to filter out recording tickets by their time properties on the server-side, e.g. not getting all of them, inspect the property and then reset them. But IMO the worker-side filter is something with a mixed track record, and I would rather tend to drop it when it's in the way. It just makes things intransparent.

[MaZderMind]

@a-tze From a bare logic standpoint it would be okay to have a "Automatic Checker Worker" which only checks Encoding-Tickets who's Encoding-Profiles have an "eligible-for-automatic-checking=yes" property.

I can't see the intransparency in this.

[a-tze]

Hmm. True. This will correlate 99.9% with the master property but an additional property on encoding profiles is not expensive and improves this thing.

[MaZderMind]

@a-tze most importantly it is explicit and does inherently document what it does and who uses it.

[pegro]

Properties on encoding profiles get propagated to all projects assigning them. So you can't have automatic-checking disabled in a project, while having it enabled in another!?

I would propose introducing a corresponding flag on the project-profile-assignment, as a database column, disabled as default. Saves us from joining even more tables?!

I also think generally properties which are inherently important for the application (excluding crs-scripts), should be stored as columns instead of properties.

[MaZderMind]

@pegro We could have a second Property, like we currently have with Publishing.YouTube.Enable=yes (on the Project) and Publishing.YouTube.EnableProfile=yes (on the Profile)

[pegro]

Are both of these properties relevant to the tracker itself? Different behavior? Showing/hiding buttons? I like to think about the tracker without having too much details of how crs-scripts (or other scripts) work in mind.

So I'm still not convinced, but if others are fine with it... ;)

[MaZderMind]

@pegro none of them are relevant to the tracker in any way. Both are solely controls for a checker-worker, that modifies ticket-states (in this case: after local inspection of the files) - not too different to the recording worker.

[pegro]

none of them are relevant to the tracker in any way.

Hm I don't think that is true.

For this feature to work, the "checking" state needs to be marked as serviceable to make the tickets available via API. Additionally, the database would need to filter in _view_serviceabletickets which tickets in _nextstate = checking are actually applicable to be assigned to a worker based on that property.

Also so far we tried to only have UI to do the non-serviceable stuff and only API for the serviceable states. This change would allow both, UI users and scripts, to do the same thing. To prevent having race conditions between UI and API, you might want to hide the "check" button in the UI and for this you again need to check this property.

Maybe I'm wrong here, but this doesn't sound irrelevant to the tracker to me.

[MaZderMind]

Additionally, the database would need to filter in view_serviceable_tickets which tickets in next_state = checking are actually applicable to be assigned to a worker based on that property.

Not necessarily. As discussed above this filtering could also be handled on the API-Level, as it is done with the recording Tickets.

But I can see you point here, that the logic might be better fittet within view_serviceable_tickets and then, yes, these Properties are indeed relevant fo the Tracker.

[pegro]

As discussed above this filtering could also be handled on the API-Level, as it is done with the recording Tickets.

Mh, if you try to handle that just on API level, then you'd have to move the ticket into a state, where it won't be handled by the checking worker again in the next loop. Otherwise this worker would get assigned a ticket to check whether to check or not to check, and if not he would reset the ticket. And if that worker is fast, UI users will need some luck that the rendered "check" buttons will actually work and not fail, due to a checker worker being assigned (admittedly a split-second) to the ticket, who does nothing but spam logs with "no, that ticket is not to be checked by me. maybe next time..." ;)

Yeah for recording ticket it would be enough to have a external trigger for the tracker, on which he would check for recording tickets to move into the next state. A separate worker for this was just the easiest way to get there when we developed that stuff. Compared to the checking worker: the recording worker is the only one touching the tickets, he gets the tickets assigned for the time of recording (so he can query for "my assigned tickets") and he doesn't reset tickets so a human can "press record".

Filtering tickets is always done in the tracker. A worker only gets a ticket assigned he is actually capable of processing.

[jjeising]

Counter-proposal: make checking an automatable state.

This was previously discussed and I'm still not convinced, actually it feels more like a strict no.

1) I like that the Tracker is responsible for state transitions, especially all transitions which don't involve any work. Yes, it won't require big changes, but then there is a script which is required to run for the Tracker to work. I don't like this idea.

2) When doing automated checking, why would this require a separate state? A checking result wouldn't change between runs, if its not a setup issue (e.g. binary missing). Therefor it can be done in encoding or postencoding, for all tickets (length too short is definitely encoding failed). Why would we want tickets (master?) without automated checking? (This would happen if some tickets are checked automatically and some by hand).

Anyway, I currently don't see any reasons to not finish and merge #171.

[pegro]

@MaZderMind I just remembered what you mean by filtered on API level. I forgot about the property filter. But still, checking would have to be marked as serviceable.

But yeah, that basic checking could be done in postencoding and make the encoding fail, if problems are detected?!