SqlServerDsc icon indicating copy to clipboard operation
SqlServerDsc copied to clipboard

BREAKING CHANGE: ReverseDSC Native Support

Open NikCharlebois opened this issue 4 years ago • 7 comments

Pull Request (PR) description

This PR introduces native ReverseDSC support for SQLServerDSC. It exposes a new Export-SQLServerConfiguration cmdlet that Reverse Engineers existing SQL Server environment and generates a resulting DSC script that is a full-fidelity representation of the existing settings on the server.

This Pull Request (PR) fixes the following issues

N/A

Task list

  • [ ] Added an entry under the Unreleased section of the change log in the CHANGELOG.md. Entry should say what was changed, and how that affects users (if applicable).
  • [ ] Resource documentation added/updated in README.md.
  • [ ] Resource parameter descriptions added/updated in README.md, schema.mof and comment-based help.
  • [ ] Comment-based help added/updated.
  • [ ] Localization strings added/updated in all localization files as appropriate.
  • [ ] Examples appropriately added/updated.
  • [ ] Unit tests added/updated. See DSC Resource Testing Guidelines.
  • [ ] Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
  • [ ] New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

This change is Reviewable

NikCharlebois avatar Sep 27 '19 02:09 NikCharlebois

Codecov Report

Merging #1434 into dev will decrease coverage by 7%. The diff coverage is 36%.

Impacted file tree graph

@@          Coverage Diff          @@
##            dev   #1434    +/-   ##
=====================================
- Coverage    98%     90%    -8%     
=====================================
  Files        38      39     +1     
  Lines      5603    6026   +423     
=====================================
- Hits       5497    5444    -53     
- Misses      106     582   +476

codecov-io avatar Sep 27 '19 05:09 codecov-io

Will work on adding Unit Tests later today

NikCharlebois avatar Sep 27 '19 11:09 NikCharlebois

@johlju wait before review. Trying to familiarize myself with the Unit Tests setup so I might submit a few extra commits. You can have a look at the general structure of the PR though to get an idea of what's coming your way :)

NikCharlebois avatar Sep 27 '19 14:09 NikCharlebois

Hey @NikCharlebois, that's a huge effort you put in, but I wouldn't be too happy putting that much code in the DSC resources, even more that the feature is not (some could argue 'yet') part of the DSC technology. Could you maybe try to have minimal code within the DSC resource's psm1s, and have the execution done in another module (i.e. under a .\modules folder).

gaelcolas avatar Oct 22 '19 07:10 gaelcolas

There are a lot of duplicated code introduced with this change, I also want to look over this to see that it actually the correct way of doing this. Please continue resolving comments on this change, but I will add an 'on hold' label for the time-being so we have chance to discuss this.

I have offline conversation with @NikCharlebois about this, but I have not have time to get back to that yet.

johlju avatar Oct 24 '19 07:10 johlju

The new CI pipeline has merged. This changed the folder structure, and also removed the dev branch. Please rebase against the master branch, and make sure to get your changes into the the file in the new location (under source folder). 😃

Read more here about the new coding workflow: https://dsccommunity.org/guidelines/contributing/#understand-the-coding-workflow https://dsccommunity.org/guidelines/testing-guidelines/#running-tests You can also use Invoke-Pester once you run build.ps1 -Task build (not documented yet) https://dsccommunity.org/guidelines/contributing/#attach-your-fork-to-a-free-azure-devops-organization

johlju avatar Jan 03 '20 09:01 johlju

Hello @NikCharlebois, I see this is the same approach used in Microsoft365Dsc too. In the early days of ReverseDSC this was done as an external "orchestrator" module, such as https://github.com/Microsoft/SQLServerDSC.Reverse, but I see that hasn't been so active recently, did you run into a problem with this approach?

kewalaka avatar Oct 25 '20 23:10 kewalaka