Opened 11 years ago

Closed 11 years ago

#565 closed defect (fixed)

Make workbench easier to use

Reported by: spaetz Owned by:
Priority: major Milestone: 0.3.3
Component: programming Keywords: review
Cc: Parent Tickets:

Description

Currently, you have to get a Workbench from the workbench manager and must not forget to destroy (delete) it yourself after processing. How hard this is?

2 of our current media_type processors (ascii and video) NEVER called workbench.destroy_workbench() which means tons of temporary files and directories were kept around.

NONE of our current media_type processors destroy the workbench in case of an exception in the processing function, e.g. on invalid images and stuff. Nice DOS attacks are to be expected, filling up our temp space :-).

Finally, we need the same boilerplate for all processing functions, getting a workbench, destroying a workbench, even in the face of exceptions etc. So I propose:

1) to make Workbench() a context manager so that we can do

with Workbench() as bench:
   bench.do_stuff....

which nicely cleans up after itself.

2) A decorator "get_workbench" which will create a Workbench, pass it to the processing function and cleans up when processing finished (even in the face of exceptions). This saves us lots of boilerplate and at least one level of indention.

Fortunately, I happen to coincidentally have a branch lying around doing just the above things.

Change History (4)

comment:1 by spaetz, 11 years ago

This is branch 565_workbench_cleanup at my repo: git://gitorious.org/~spaetz/mediagoblin/spaetz-mediagoblin.git

comment:2 by Elrond, 11 years ago

spaetz is doing all the stuff hanging around in my brain!

I was considering to pass in the workbench into each media processor.

ping me later to review your branch!

comment:3 by spaetz, 11 years ago

Keywords: review added

I just updated the branch to current master. It also includes a test for the workbench function.
Note that the last patch in the series also partially addresses #419 (don't read all VIDEOS in RAM)

Version 0, edited 11 years ago by spaetz (next)

comment:4 by Christopher Allan Webber, 11 years ago

Resolution: fixed
Status: newclosed

I have merged most of the branch. There's one more commit in it which is:

c00e18f * Do not read complete videos in RAM (#419)

I will save that for #419.

Good work! This definitely makes the workbench much, much nicer in many ways.

Note: See TracTickets for help on using tickets.