[POC] Remap

[PietroPasotti]

This PR adds to scenario.State a few utility methods for manipulating state objects.

State.remap

"get me the corresponding object"

use case:

You create an object (relation, storedstate, secret, network, storage...), put it in State along with potentially many others
You run scenario
You want to retrieve the 'same' object and see how it was modified by the charm
However scenario's objects are immutable, so you can't do that easily without manually filtering all objects...

Current code:

relation = scenario.Relation("foo", local_app_data={"foo": "bar"})
state = scenario.State(relations=[relation])

state_out = context.run(...)
relation_out = [r for r in state_out.relations if r.relation_id == relation.relation_id][0]

with remap:

relation = scenario.Relation("foo", local_app_data={"foo": "bar"})
state = scenario.State(relations=[relation])
  
state_out = context.run(...)
relation_out = state_out.remap(relation)

State.patch

"modify just this component"

Often you want to dynamically create 'variations' of a State:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)

relation_but_different = relation.replace(local_app_data=B)

# copy over all relations except the one we're changing, inject that one separately
state_but_different = state.replace(relations=[r for r in state.relations if r.name != "foo"] + [relation_but_different]) 

With State.patch:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)
state_but_different = state.patch(relation, local_app_data=B)

# but also:
state_but_different = state.patch(relation, local_app_data=B).patch(secret, revisions={...}).patch(network, ingress_address=...).patch(...)

(this could also be used to do delta-based comparisons):

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)

state_out = context.run(...)

# assert that the only thing that has changed is relation foo's local app databag
assert state_out == state.patch(relation, local_app_data=B)

# but also:
assert state_out == state.patch(relation, local_app_data=B).patch(secret, revisions={...}).patch(storage, ...)

State.insert

In order to dynamically create modified copies of a state, right now you're limited to using the replace api. Suppose you want to add a relation:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)

state_but_different = state.replace(relations=state.relations + [Relation(...)])

with insert:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)
state_but_different = state.insert(Relation(...))

# but also:
state_but_different = state.insert(Relation(...)). insert(Secret(...)).insert(StoredState(...)).insert(Storage(...))

State.without

"Remove just this object"

counterpart to insert but removing stuff.
with without:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)
state_but_different = state.without(relation)

assert relation not in state_but_different

# but also:
state_but_different = state.without(relation). without(secret).without(storage).without(storedstate)

[tonyandrewmeyer]

Current code:

relation = scenario.Relation("foo", local_app_data={"foo": "bar"})
state = scenario.State(relations=[relation])

state_out = context.run(...)
relation_out = [r for r in state_out.relations if r.relation_id == relation.relation_id][0]

For what it's worth, as long as you have the original relation/relation ID, then you can just do this:

relation_out = state_out.get_relation(r.id)

I've seen tests where the state comes as a fixture and so the original relation isn't conveniently available, but remap would have the same issues in that case, I think.

with remap:

relation = scenario.Relation("foo", local_app_data={"foo": "bar"})
state = scenario.State(relations=[relation])
  
state_out = context.run(...)
relation_out = state_out.remap(relation)

It seems like the main advantage over get_relation is that you can just always use remap rather than having to pick between get_relation, get_container, and so on. It's also potentially more convenient to pass the object than its identifier.

Overall, I'm -0 on remap.

State.patch

"modify just this component"

Often you want to dynamically create 'variations' of a State:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)

relation_but_different = relation.replace(local_app_data=B)

# copy over all relations except the one we're changing, inject that one separately
state_but_different = state.replace(relations=[r for r in state.relations if r.name != "foo"] + [relation_but_different]) 

This needs to use dataclasses.replace now, although it's roughly the same other than needing the import:

relation_but_different = dataclasses.replace(relation, local_app_data=B)
state_but_different = dataclasses.replace(state, relations=[r for r in state.relations if r.name != "foo"] + [relation_but_different]) 

With State.patch:

I'm +1 on patch.

State.insert

Minor nit: probably this should be State.add since they're sets not lists now?

+1 on insert (or add).

State.without

For what it's worth, I think State.remove would be more consistent.

"Remove just this object"

counterpart to insert but removing stuff. with without:

relation = Relation('foo', local_app_data=A)
state = State(relations=[relation, ...], ...)
state_but_different = state.without(relation)

assert relation not in state_but_different

I'm pretty certain this could be:

relation = Relation('foo', local_app_data=A)
state = State(relations={relation, ...}, ...)

assert relation not in state.relations

That seems simpler to me.

# but also:
state_but_different = state.without(relation). without(secret).without(storage).without(storedstate)

And similar here, I think I would rather do:

assert relation not in state.relations
assert secret not in state.secrets
assert storage not in state.storages
assert storedstate not in state.stored_states
# or
assert relation not in state.relations and secret not in state.secrets and storage not in state.storages and storedstate not in state.stored_states

So -1 on without unless there's something I'm missing here.

Edit: I guess the better use-case is not asserting whether something is still in the state (e.g. did a secret get removed) but setting up a state for another event. I suppose the symmetry with adding/inserting is nice in that case. So I guess I'm -0 on without.

[benhoyt]

I'm not a fan of these. I'd much rather have a little bit of boilerplate that any Pythonista can understand, rather than starting to create our own little domain-specific language. Specifics:

• Remap: I find the name (and somewhat the operation) of remap confusing. And as Tony pointed out, that's why we added that get_container(r.id) and similar methods, which also have simpler type signatures.
• Patch: just use dataclasses.replace or explicitly recreate the object. If anything, we could add State.replace to make it a little shorter: state.replace(foo=bar) rather than dataclasses.replace(state, foo=bar).
• Insert: it seems to me you can just use dataclasses.replace with + or | on the field. Tiny bit longer, but more obvious and explicit.
• Without: just recreate the state object, or use dataclasses.replace again and filter.

I could maybe see myself coming around to State.replace as a shorthand for dataclasses.replace, but I'm not a fan of adding a bunch of methods.

However, to make a more informed decision, I'd rather see this in the context of actual, real-world tests, to see what it's actually saving, and whether there are other/better ways to achieve the same thing.

[benhoyt]

Per discussion today, we're going to leave this for now, and see what patterns start appearing (and requests start coming) from teams that start writing state-transition / Scenario tests. So closing this for now, and we can always revisit.

In addition, we'd like to recommend that developers using Scenario normally create new objects, rather than replace/patch existing ones. This might mean structuring tests and fixtures a bit differently when DRYing things up, but there are reasonable ways to do this.