(FM-5324) Fix TSQL error propagation #173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- This method only appears in tests
- Minor refactor to make the return value more obvious
- This method is extraneous / confusing. Better to refactor all code to execute with the providers `run` method
- Introduce a minor refactor so that the `returns` property captures errors properly from TSQL executions. Previously, if a `refresh` occurred against a sqlserver_tsql the output would cause a failure, but simply executing TSQL would not do the same. Follow the same model as the Puppet exec type (where much of this code comes from), and have `refresh` trigger the `sync` method of the `results` property, so that the code is now shared.
- The command method for SqlConnection only has a single caller, the block form is never used and the result is not either. Furthermore, the caller open_and_run_command, already catches win32_exception, so there's more code here than necessary.
- The existing code only has one code path, so remove the unneeded method.
- Tests should be better (self) documented
- Properly parse errors that look like: SQL Server error message instead of those that must have whitespace like: SQL Server error message
- The existing SqlConnection class has a public property exception_caught, which is not used. Further, it uses this value to save off exceptions inside the class, rather than simply returning the parsed errors when open_and_run_command is executed, which is presently the only caller that ever looks at the error values. This provides an opportunity to simplify the code paths, and remove unnecessary caching of errors and the 2 unnecessary methods has_errors and error_message from the SqlConnection class. Instead, directly return a new ResultOutput instance as soon as SQL code is executed. Tests are modified accordingly.
- Previously, there was no way of getting the log output from SQL Server when executing TSQL during an onlyif. Only the status code was emitted, which is not helpful when trying to debug.
- These tests were previously stubbing return values instead of asserting that the code was behaving properly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor SqlConnection / tests to simplify code.
Fix error propagation in
SqlConnection