Enhanced unified function to handle errors, added NoWait and fix endless loop for LRO

[NowinskiK]

Pull Request

Fixes #131
Fixes #123

Pull Request (PR) description

• Removed Revision History from Get-FabricSQLDatabase, Get-FabricSQLDatabase
• Added NoWait switch parameter to New-FabricSQLDatabase
• Enhanced logic in unified function Test-FabricApiResponse to handle API results and moved it to private functions
• Remove-FabricSQLDatabase uses unified function to handle API results
• Fixed bug in Get-FabricLongRunningOperation - Uri was incorectly created
• Fixed bug in Get-FabricLongRunningOperationResult - uses correct statusCode
• Applied splatting for several parameters in Invoke-FabricRestMethod and output results in debug mode

Task list

• The PR represents a single logical change. i.e. Cosmetic updates should go in different PRs.
• Added an entry under the Unreleased section of in the CHANGELOG.md as per format.
• Local clean build passes without issue or fail tests (build.ps1 -ResolveDependency -Tasks build, test).
• Comment-based help added/updated.
• Examples appropriately added/updated.
• Unit tests added/updated..
• Integration tests added/updated (where possible).
• Documentation added/updated (where applicable).
• Code follows the contribution guidelines.

[github-actions]

Linux Test Results

4 864 tests   4 863 ✅  41s ⏱️
  591 suites      1 💤
    1 files        0 ❌

Results for commit d317037.

♻️ This comment has been updated with latest results.

[github-actions]

WinPS51 Test Results

5 068 tests   5 067 ✅  55s ⏱️
  592 suites      1 💤
    1 files        0 ❌

Results for commit d317037.

♻️ This comment has been updated with latest results.

[github-actions]

WinPS71 Test Results

5 068 tests   5 067 ✅  53s ⏱️
  592 suites      1 💤
    1 files        0 ❌

Results for commit d317037.

♻️ This comment has been updated with latest results.

[SQLDBAWithABeard]

So that you understand, this is one of my bug bears and I am frequently on video saying that I will call it out publicly when I see it so I am duty bound to do this :-)

Please do not use the commit message for the what was done as that does not provide value.

To summarise an hour long session, The commit message should be the why it was done or the intended result of the change so that the commit history is of use to future you and your team. I am not asking you to change the three commits that you have done in this PR but please, from this point on.

[jpomfret]

Hey @NowinskiK - some good updates, thanks for the PR!

Just a reminder though to try and keep each PR small and representing one logical change, not always easy I know! But it's ok to open multiple PRs for different things - just makes reviewing and testing easier.

[SQLDBAWithABeard]

@NowinskiK - please read the comment about commit messages.

[NowinskiK]

Hey @NowinskiK - some good updates, thanks for the PR!

Just a reminder though to try and keep each PR small and representing one logical change, not always easy I know! But it's ok to open multiple PRs for different things - just makes reviewing and testing easier.

@jpomfret, yes, I know, I remember and agree. Believe me, 😊 I already minimised the scope of this PR as I was working on new sets of features when I noticed this error. The other functions in this change set are mainly for testing the fixed approach.
I will try to do this better next time!

[NowinskiK]

So that you understand, this is one of my bug bears and I am frequently on video saying that I will call it out publicly when I see it so I am duty bound to do this :-)

Please do not use the commit message for the what was done as that does not provide value.

To summarise an hour long session, The commit message should be the why it was done or the intended result of the change so that the commit history is of use to future you and your team. I am not asking you to change the three commits that you have done in this PR but please, from this point on.

Read this. Makes sense! I will do my best and improve moving forward.

[tiagobalabuch]

1. In the Test-FabricApiResponse, there will be LROs with no results, so we need to consider how to handle these scenarios (I got stuck on this and my solution was to add a new parameter. Let's work together to see if there's a better approach). Otherwise, you'll end up with an object that has an error column saying there's no result.
2. Now, there are two calls that must be made in a function: Invoke-FabricRestMethod and Test-FabricApiResponse. This means whenever a new function is developed, it must call Test-FabricApiResponse, and people might forget. My suggestion is to make this call inside Test-FabricApiResponse. That way, there's only one place to change in the future if needed.

[NowinskiK]

@tiagobalabuch thanks for the input.
ad.1) Could you elaborate, please, and give an example?
ad.2) Sure. I was thinking about that in the next iteration. I will remain Invoke-FabricRestMethod as main function to execute, and add Test-FabricApiResponse at the end. I hope you put typo over there and you meant "make this call inside Invoke-FabricRestMethod" (not Test-FabricApiResponse).
I will make the run of Test optional, so all existing functions will not run it by default, so they can handle it themselves.

[tiagobalabuch]

@NowinskiK and I had a call and decided to keep two separate functions for now. For LRO with no results, we'll leave it as is since the function currently returns the results.