Galicaster icon indicating copy to clipboard operation
Galicaster copied to clipboard

Adhoc enqueue params

Open paulgration opened this issue 7 years ago • 5 comments

This moves serializing/deserializing of enqueue_params property values for media packages to the setProperty and getProperty methods of mediapackage.py. It also ensures that wherever an enqueue_job or do_job call is made, the enqueue_params are passed along the chain until it reaches the point at which to ingest the media package.

In fixing this, it is possible to specify a target workflow and workflow_parameters per media package (rather than use the workflow specified in conf.ini) for manual (adhoc) recordings.

The code for ingesting and specifying the workflow is the _ingest method of galicaster/core/worker.py. Manual and scheduled recordings are handled differently:

  • Manual ingest uses the params parameter passed to the method (via jobs that are either queued or executed immediately)
  • Scheduled recordings can include a org.opencastproject.capture.agent.properties to specify a workflow definition

Currently (2.0.x), there are only two places that media package properties are set:

I had a requirement to do the same as the latter within a plugin, e.g. media_package.setProperty('enqueue_params', {'workflow': 'test'}). Rather than import json and serialize/deserialize everywhere that the enqueue_params property needs to be set, this PR ensures that all enqueue_params properties are serialized/deserialized as part of the setProperty/getProperty methods of mediapackage.py. All other properties that are set remain as strings. So the only change here is to make sure that the enqueue_params property values are in a consistent format.

The managerui makes calls to the worker do_job and do_job_nightly methods but didn't pass the params parameter on to these methods so now the enqueue_params property is fetched and passed with these calls. Similarly the recorder service enqueue_ingest didn't pass any params down the line so the same change has been made here.

Generally, storing JSON representations of Python objects within a text node of XML doesn't seem like the best way to do this but for now I've gone for the path of least resistance to get this working - perhaps this file could be .json in future...?

~~Unfortunately, tests already appear to fail in 2.0.x (before this PR) when the manual config option of [ingest] is set to either nightly or immediately so testing this one isn't particularly easy~~. Tests pass for this but I've also been doing a mix of printing debug statements in _ingest, setProperty, getProperty, reviewing content written to the galicaster.xml file and using a test plugin to start the recording:

from gi.repository import Gtk
from galicaster.core import context
from galicaster.mediapackage.mediapackage import Mediapackage

dispatcher = context.get_dispatcher()
recorder = context.get_recorder()
repository = context.get_repository()

def init():
    dispatcher.connect('init', on_init)
    dispatcher.connect('recorder-stopped', on_recorder_stopped)

def on_init(_source):
    RecordDialog()

def on_recorder_stopped(_source, media_package_id):
    media_package = repository.get(media_package_id)
    print media_package.getProperty('enqueue_params')
    RecordDialog()

class RecordDialog(object):
    def __init__(self):
        parent = context.get_mainwindow().get_toplevel()
        self.dialog = Gtk.Dialog()
        self.dialog.set_title('Test')
        self.dialog.set_default_size(200, 100)
        self.dialog.set_transient_for(parent)
        self.dialog.set_modal(True)

        record = Gtk.Button('Record')
        record.set_hexpand(True)
        record.set_vexpand(True)
        record.connect('clicked', self.on_record_clicked)

        box = self.dialog.get_content_area()
        box.add(record)
        self.dialog.show_all()

    def on_record_clicked(self, _source):
        media_package = Mediapackage(title='Test passing params')
        media_package.setProperty('enqueue_params', {'workflow': 'test'})
        recorder.record(media_package)
        self.dialog.destroy()

paulgration avatar Jan 31 '18 16:01 paulgration

Updated tests since 2071b23be so that they no longer fail with this PR

paulgration avatar Feb 05 '18 12:02 paulgration

this is definitely a piece of work that has value. i've found myself this division between manual properties and properties that come from opencast frustrating.

i'll take a look at this PR next

andiempettJISC avatar Nov 13 '18 15:11 andiempettJISC

I'm wondering whether this should be targeted at master now..? And if it is, whether anything needs to change about the json imports because since creating this PR there has been other commits that have changed the storing of data from galicaster.xml to galicaster.json so it may be that json.dumps isn't required any longer? I haven't look into it in detail though

paulgration avatar Nov 14 '18 11:11 paulgration

ah yeah i missed that. see https://github.com/teltek/Galicaster/blob/master/CONTRIBUTING.md. so use master

please feel free to change this to master as i don't think it could be accepted against 2.0.x

i think it is safe to still use json.dumps as i've tested this against latest master and its working so far. its probably a larger piece of work to look at how Galicaster stores temporary data etc

andiempettJISC avatar Nov 14 '18 11:11 andiempettJISC

ah yeah i missed that. see https://github.com/teltek/Galicaster/blob/master/CONTRIBUTING.md. so use master

please feel free to change this to master as i don't think it could be accepted against 2.0.x

i think it is safe to still use json.dumps as i've tested this against latest master and its working so far. its probably a larger piece of work to look at how Galicaster stores temporary data etc

Ok, that sounds promising then. I just changed target branch to master

paulgration avatar Nov 14 '18 13:11 paulgration