microsoftgraph / microsoftgraph/msgraph-sdk-powershell

Exactly what are you trying to do in your auto-generated modules?

Offen
#372 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

  • #766 von @georgend — ohne Merge geschlossen
AutoREST-dependency
Vorherrschende Sprache
C#
Sterne
898
Forks
230
Ø Merge
2 T. 5 Std.
Gemergte PRs (30 T.)
31

Beschreibung

Discover of issue #371 led to me digging into your module internals more, and I think you need to re-think what you are doing with your auto-generated modules.

Let's take Microsoft.Graph.Users.User version 0.9.1 as an example.

It contains a top-level module that is defined using a psd1 file, a dll, and multiple psm1 files.

The psd1 file identifies the psm1 file as the root module, but it does not identify the dll as a nested module to load as part of the NestedModules entry in the manifest. Instead it leaves the psm1 file to manually load the dll.

Issue: There is no need to manually load a nested binary module like this.
Recommendation: Simply add the relative path (e.g. './bin/Microsoft.Graph.Users.User.private.dll') to the NestedModules entry in the manifest, and PowerShell will load it for you. That's what NestedModules is for!

Once the binary module is manually loaded, the main psm1 file loads a custom psm1 script module that in turn loads an internal psm1 script module. The custom psm1 script module then goes on to disable implicit exports, and then dot source any nested ps1 files that are in the custom folder.

Issue: You don't have any nested ps1 files in custom folders in any of the Graph modules. None.
Recommendation: Maybe this is some legacy code? It's just more processing for nothing though, so if it's legacy, remove that logic. If not, comment it out until you need it. No need to do file I/O where it is not required.

The custom psm1 file then goes on to export the functions and aliases that are defined at that point, which will make them visible to the parent module.

The internal psm1 file switches the contents of a subfolder based on the Graph profile that was selected, and then reads all ps1 files in that profile subfolder to get the command definitions for that module. It then exports the functions and aliases, which will make them visible to the parent module.

Going back to the top-level psm1 file, it also reads the profile settings and loads the cmdlets from the exports path.

Issue: Why does it take three psm1 files to do what one psm1 file could do for you? There doesn't seem to be any need for your custom psm1 file, nor does there seem to be any need for your internal psm1 file. Your top-level psm1 file can do (and is already doing!) what is needed without the others adding extra work to the loading of these modules. Sure enough, if I look at the information stream output, I see multiple duplicate messages that reinforce that there is unnecessary duplication being done here.
Recommendation: Reel this back in to just one psm1 file, one psd1 file, and one dll, to keep things performant and eliminate duplication of effort. You don't need more than a single psm1 file with what you have today. Auto-generated code can and should be clean.

Other miscellaneous issues:

  • You're using pipelines in your psm1 files where they are not necessary. Pipelines add overhead to processing times. They should be used when processing large amounts of data that you don't have in memory. They should not be used when you have data already in memory. Examples taken from Microsoft.Graph.Users.User.psm1:

    Instead of this Do this Reason
    $directories = Get-ChildItem -Directory -Path $exportsPath $directories = @(Get-ChildItem -Directory -Path $exportsPath)` This is necessary for the changes recommended below
    if(($directories | ForEach-Object { $_.Name }) -contains $instance.ProfileName) { if($directories.foreach('Name') -contains $instance.ProfileName) { Collection is already in memory (performance), code is easier to maintain (simplicity)
    $profileDirectory = $directories | Where-Object { $_.Name -eq $instance.ProfileName } $profileDirectory = $directories.where{$_.Name -eq $instance.ProfileName} Performance and simplicity, as above
    } elseif(($directories | Measure-Object).Count -gt 0) { } elseif ($directories.Count -gt 0) { No pipeline (performance) and and simplicity again
    $profileDirectory = $directories | Select-Object -Last 1 $profileDirectory = $directories[-1] No pipeline (performance) and simplicity again
    Get-ChildItem -Path $exportsPath -Recurse -Include '*.ps1' -File | ForEach-Object { . $_.FullName } `foreach ($file in Get-ChildItem -Path $exportsPath -Recurse -Include '*.ps1' -File) {. $_.FullName } Collection is small, faster to process using foreach keyword (performance)

Last, in your psd1 file you are still using wildcards for AliasesToExport. When a single module does hit, it results in a performance hit for all of PowerShell because command discovery is affected. Please, please replace AliasesToExport with the actual list of aliases that are exported, or an empty array if there are none. Ditto for FunctionsToExport and CmdletsToExport.
AB#7392

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginnen Sie mit den generierten Dateien Microsoft.Graph.Users.User.psd1 und Microsoft.Graph.Users.User.psm1 und untersuchen Sie anschließend die in der Issue beschriebenen benutzerdefinierten und internen psm1-Dateien. Vergleichen Sie die Logik zum Laden von Modulen, zum Exportieren, zur Profilauswahl und für Pipelines mit den Empfehlungen. Die Aufgabe ist erledigt, wenn generierte Module die einfachere Manifest- und Lad 구조 verwenden, unnötige Verarbeitung vermeiden und explizite Exporte auflisten.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
powershell
Bereich
tooling
Issue-Typ
Refactoring
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Veraltet
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
25/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.