microsoft / durabletask-mssql

Microsoft SQL storage provider for Durable Functions and the Durable Task Framework
MIT License
87 stars 32 forks source link

Added multi-schema support for DTF Sql-Server #110

Closed AndreiRR24 closed 2 years ago

AndreiRR24 commented 2 years ago

Added multi-schema support for DTF Sql-Server as requested in #105. Following the plan suggested by George, I've refactored the code to be able to use a custom schema name instead of the hardcoded "dt" as such:

As such, this version is backwards compatible as far as I know and is able to do all the operations with custom schema names.

Co-Authored-By: George Moldovan george.moldovan@uipath.com

ghost commented 2 years ago

CLA assistant check
All CLA requirements met.

cgillum commented 2 years ago

Awesome! Looking forward to going through this. I’m out of the country on vacation now but will do a full review when I get back in a few weeks.

AndreiRR24 commented 2 years ago

While writing the test @cgillum suggested, I encountered a problem with the LogAssert: After creating the first schema and validating the following steps, image the second schema validation fails with the following error image even though after manual testing, the logic is right and the program is executing the flow correctly. This can be avoided by not running the second validation, but this doesn't sit quite right. Do you have any suggestions in solving this?

On another note, I do not know how to edit the documentation. I am guessing you mean this one and not the README.md file.

cgillum commented 2 years ago

On another note, I do not know how to edit the documentation. I am guessing you mean this one and not the README.md file.

Oh, sorry for not explaining this! Yes, I meant the published documentation. This documentation is authored via markdown files under the /docs directory. We're using GitHub Pages to have them published.

cgillum commented 2 years ago

I'm not sure about the validation error. The log validation framework was created by another contributor @wsugarman, so I don't have as much insight into how it works internally to know why you're seeing these unexpected failures.

wsugarman commented 2 years ago

While writing the test @cgillum Chris Gillum FTE suggested, I encountered a problem with the LogAssert: After creating the first schema and validating the following steps, image the second schema validation fails with the following error image even though after manual testing, the logic is right and the program is executing the flow correctly. This can be avoided by not running the second validation, but this doesn't sit quite right. Do you have any suggestions in solving this?

On another note, I do not know how to edit the documentation. I am guessing you mean this one and not the README.md file.

I haven't looked at this in awhile haha, but it seems that there is a missing log entry that was expected: "secondTestSchemaName._UpdateVersion". In short, the tests record the log entries in a list, and the extensions validate the contents of the log through the Expects methods: https://github.com/microsoft/durabletask-mssql/blob/main/test/DurableTask.SqlServer.Tests/Logging/LogAssertExtensions.cs

AndreiRR24 commented 2 years ago

Updated the code with @cgillum's suggestions.