CommandContextManager is misleading

[smithmx]

First, this is not allowed:

https://github.com/dolittle-runtime/Runtime/blob/d1c59c5546ac53a1d4ffe5fa03e0ce4cc80a32a4/Source/Commands.Coordination/CommandContextManager.cs#L13

You cannot use ThreadStatic in the context of a web application. At best it should be AsyncLocal.

However, a bigger problem is the idea of enlisting multiple commands within a single command context. A command is a transaction, you cannot have multiple commands, effectively multiple transactions, within a single context. Every command should create its own context.

┆Issue is synchronized with this Asana task

[jakhog]

Just for correctness - this file has now been moved to SDK as part of the v5 changes. So the new location is here: https://github.com/dolittle-runtime/DotNET.SDK/blob/90d4657f0ada16f6462bb8f93e56ef1c97fc3af8/Source/Commands.Coordination/CommandContextManager.cs#L13

[jakhog]

The ThreadStatic problem has been fixed in dolittle/DotNET.Applications@4f81d96.

As for multiple commands being enlisted in a single command context, I don't really understand the problem you are describing. If you establish a CommandContext for a new command - i.e. not the Command that was used to establish the current CommandContext, it will create a brand new one. So there will only ever be a single Command in a CommandContext, as you can also see here: https://github.com/dolittle-runtime/DotNET.SDK/blob/5.0.0/Source/Commands.Coordination/CommandContext.cs.

So could you please clarify what you mean with the second part of your issue?

[smithmx]

Ah, ok. I could have been clearer.

There can be no nesting of commands so there's no need for a command context manager that keeps track of nested commands. The only use for the CommandContextManager would be to enable this as far as I can see and we definitely don't want that.

Neither is there, for that matter, use for a command context that can track more that one aggregate.