New file interface - take 2

[ikelos]

Ok, a bunch of changes now. We're now also identical to a file interface, apart from the final filename is changeable up until the point where the file is closed. This is no longer a hard and fast requirement (dlllist which used to append the pid, now hands it in as a prefix), so we could essentially make it permanently read-only (and I'd be happy with either way, given it can't really be used as a context manager if you're going to the change the name later, because it would have been closed and the filename finalized by that point).

There's no longer a separate commit step, meaning that technically UIs can do with the data what they'd like as soon as they get it. The CLI has a couple of implementations, one of which is the equivalent of a BytesIO, and the other is the equivalent of a file (it gets written to a temporary file in the same directory, not just to allow finalizing the filename later, but also to make the file writes appear atomic, so that partial data can't be returned).

Reasonably happy with this, I've played with the nomenclature several times. I've gone with file_handler for the class which is essentially a file, but also a context manager, file_data for the thing you actually write to, and file_output for the filename/error message the plugin responds with. Again, up for grabs, but hopefully the biggest of the decisions required...

As per the previous one, no time limit on this, but given how impending our need to have gotten this right is, it would be good to get it reviewed as soon as possible. I've also updated volumetric to use the new file mechanism (rather than using the BytesIO implementation, I instead write out to disk and haven't found a clean way to ensure all temporary files are removed at shutdown, so this can fill the filesystem and therefore volumetric is still a toy and not production worthy).

@npetroni If you could give this a look over and let me know what you think? I'd quite like to get this right because (as shown by the version numbers changing for every single plugin), altering the core API makes all previously written plugins (potentially) break, require a version bump and therefore shouldn't be done often or lightly...

[ikelos]

Let me know if you want me to re-issue this with the commits in a clearer order/with the fixes squashed? Most of the plugin files can be ignored, but the CLI and the interfaces/plugins.py are the important ones. Plugins with more significant change *should just be refactoring to make the file dumping easier, but I might've folded stuff in that I shouldn't (and as I say, happy to redo it in that case). 5:)

[npetroni]

Is it possible that volshell is broken in this branch? Should the dependency be bumped to (2,0,0)?

[npetroni]

Is it possible that volshell is broken in this branch? Should the dependency be bumped to (2,0,0)?

Hmm, actually it seems like maybe it's another version requirement that's failing.

[ikelos]

Ok, so this has now been rebased and a few fixes pushed to it. The first uses self.open rather than self._file_handler. This may be somewhat confusing, because we're essentially using a class constructor as a method that returns an open (similar to open). So the open_method's type is actually a specific class, but I'm not sure how to clear up that confusion. I've removed the with clauses, leaving the file handles open on return. They will explicitly need closing, but this allows plugins calling a classmethod to make changes to the final preferred filename, before it's committed to the UI for processing, which may be useful. This may also lead to plugin authors scratching their head over why the file doesn't get committed, but hopefully they'll figure out it's because the file was never closed. I've also fixed volshell which missed the required framework version bump.

This is a very big change and I doubt I'll have caught all the little gotchas, but we can fix them up after the change is made. Just one more eyes over will suit me and I'll merge it then...

[npetroni]

I'm still having trouble running volshell, has that been resolved?

[ikelos]

Yep, I've now resolved the volshell issues (hopefully), thanks for double checking. 5:)

[npetroni]

Yep, I've now resolved the volshell issues (hopefully), thanks for double checking. 5:)

Cool, seems to work now, thanks.

[ikelos]

If you're happy with it, I'll merge it (and we can fix up breakages people find as of tomorrow)... 5:P

[npetroni]

Not necessary related to this PR directly, but I tried using vol.py -o outdir and when outdir doesn't already exist I end up with a lot of zero-length files with names like tmp_zte5nrw3.vol3. When outdir does exist, everything seems to go fine.

[npetroni]

Actually, when I run windows.dlllist I end up with a few zero-length tmp_qlk16uw1.vol3 in outdir. Other files have the correct format pid.396.USER32.dll.0xe03650.0x775b0000.dmp.

[ikelos]

The missing directory we can definitely fix, but I agree they're probably not for this PR. Could you file a separate issue and I can get that fixed in there please?

[npetroni]

Sounds good. Looks good to me.