HomePhabricator

Introduce BatchCommandRecords class
5c16c5d3166cUnpublished

Tags
None
Referenced Files
None
Subscribers
None

Unpublished Commit · Learn More

Publishing Disabled: All publishing is disabled for this repository.

Description

Introduce BatchCommandRecords class

Using a generic MutableSequence for an OpenBatch’s command_records only
gets us so far, and I think we’ve reached the point where it’s time to
swap it out with a more specific class. There are two implementations,
one wrapping a plain list (used by InMemoryBatchStore) and one accessing
the database, corresponding to the previous _DatabaseCommandRecords.

This already clarifies several signatures. Command records can only be
retrieved in slices, not by individual index (the ability to retrieve
them by index was only used in one test, where it was easily removed).
On the flipside, replacement is only supported for single commands, and
also only if the value is a CommandFinish. There are no methods for
inserting or deleting command records. We can also stop using slice
objects and instead directly use offset and limit everywhere, which is
what we were effectively doing anyways.

Additionally, this class is a good place for a future method to
atomically transition commands from the “planned” to a new “pending”
state, preventing multiple actors from trying to run the same command
plan simultaneously. I’m not sure if this is currently possible via
multiple Flask threads, but it’s much more likely to become a problem
once running of batches in the background is added.

Details

Provenance
LucasWerkmeisterAuthored on Mar 24 2019, 7:08 PM
Parents
R2494:c4ba7cca422c: Fix errors on nochange edit response
Branches
Unknown
Tags
Unknown
ChangeId
None