- 
                Notifications
    You must be signed in to change notification settings 
- Fork 778
[1.16] Adds workflow limitations #4865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Updates state store table to include supported stores. Adds a warning emoji to state stores which have workflow limitations. Adds workflow limitation sections to the cosmosDB & dynamodb setup docs. Signed-off-by: joshvanl <me@joshvanl.dev>
| @JoshVanL - Also looks like there is a failing link in the CosmosDB file. Should not have localization. | 
Signed-off-by: joshvanl <me@joshvanl.dev>
| @msfussell PTAL | 
        
          
                ...ocs/content/en/reference/components-reference/supported-state-stores/setup-azure-cosmosdb.md
          
            Show resolved
            Hide resolved
        
              
          
                ...ocs/content/en/reference/components-reference/supported-state-stores/setup-azure-cosmosdb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ocs/content/en/reference/components-reference/supported-state-stores/setup-azure-cosmosdb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | Just my 2c, I don't believe we should introduce a limited-support status for Workflows. It should either be a simple yes or no. I can easily foresee some users adopting Workflows (on limited components), which may work initially for small use-cases, but then over the course of time usage increases and then they start bumping up against these limitations in production, causing unexpected problems, which is never good for building the credibility of Dapr Workflows. What would our response be to this? "We told you so. RTM" Nope. Thats not a good look IMO. We don't have any tooling to allow users to port their operational data from one State Store to the other, so we have no easy off-ramp from a limited-support State Store onto one that is fully supported. Not good. Some of these limitations are immovable, like CosmosDb for example. It feels like we're setting up users for future failure by leading them down a path that we know we can't support them on, rather than allowing users to fall into the pit of success. Thought experiment : Name one other stable Building Block where we give mixed signals about its ability to adhere to the Abstraction of what the Building Block provides, on a per component basis? I don't believe we do? So why are we starting now? Happy to be wrong of course. | 
| @olitomlinson I disagree- like all software, there are limitations on what it can do based on the technology or hardware. This is is true for all systems, so I think it is entirely appropriate that these limitations are exposed to the user. A user may already be using one of these technologies, and their use case entirely fits into these limitations. By not offering this as a store, we unnecessarily scope down Dapr's feature set. The state store building block has all kinds of limitations on API, and again, the raw limitation of the db technology again. | 
| 
 Sure, there are always limits, but this is a sneaky one! My main concern is that there is no off-ramp here. Workflows has taken enough hammer over the last few months for not having good operational tooling, and now we're handing over another footgun. 
 I don't see how we're scoping down the feature set? The features stay the same. We're limiting the audience for sure though. 
 Yeah, having CosmosDb as a transactional compliant State Store component, but has a low upper bound of 100, is a mess, and I'm sure knowing what we know now we would make stricter requirements on what constitutes transaction support (see Redis) We're always going to hit unknown unknowns, its unavoidable. But here we are making a decision on something we absolutely do know about. Sure adopters use-cases might be fine today, but what about then they want to do more with Workflows? We can't support two actor state stores being online at one time, so customers will have to do a full offline migration, for which we offer no operational tools. It's not a good outlook. | 
Co-authored-by: Mark Fussell <markfussell@gmail.com> Signed-off-by: Josh van Leeuwen <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
table Signed-off-by: joshvanl <me@joshvanl.dev>
| @olitomlinson I have remove the warning from the table to instead mark dynamoDB and cosmosDB as not supported, but have left the verbiage in the components docs pages respectively. | 
        
          
                daprdocs/content/en/developing-applications/building-blocks/workflow/workflow-architecture.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ocs/content/en/reference/components-reference/supported-state-stores/setup-azure-cosmosdb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                ...ocs/content/en/reference/components-reference/supported-state-stores/setup-azure-cosmosdb.md
          
            Show resolved
            Hide resolved
        
              
          
                daprdocs/content/en/reference/components-reference/supported-state-stores/setup-dynamodb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                daprdocs/content/en/reference/components-reference/supported-state-stores/setup-dynamodb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                daprdocs/content/en/reference/components-reference/supported-state-stores/setup-dynamodb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …tate-stores/setup-azure-cosmosdb.md Signed-off-by: Mark Fussell <markfussell@gmail.com>
Co-authored-by: Mark Fussell <markfussell@gmail.com> Signed-off-by: Josh van Leeuwen <me@joshvanl.dev>
Signed-off-by: joshvanl <me@joshvanl.dev>
| @msfussell updated, PTAL | 
Signed-off-by: joshvanl <me@joshvanl.dev>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged two grammar issues. @olitomlinson - Any comment?
        
          
                ...ocs/content/en/reference/components-reference/supported-state-stores/setup-azure-cosmosdb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
              
          
                daprdocs/content/en/reference/components-reference/supported-state-stores/setup-dynamodb.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      …tate-stores/setup-azure-cosmosdb.md Signed-off-by: Mark Fussell <markfussell@gmail.com>
…tate-stores/setup-dynamodb.md Signed-off-by: Mark Fussell <markfussell@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Updates state store table to include supported stores.
Adds a warning emoji to state stores which have workflow limitations.
Adds workflow limitation sections to the cosmosDB & dynamodb setup docs.