Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate cmdlets from forked AutoREST.PowerShell repository. #2542

Merged
merged 27 commits into from
Jan 29, 2024

Conversation

timayabi2020
Copy link
Contributor

@timayabi2020 timayabi2020 commented Jan 24, 2024

Fixes #1570 and #2476

Changes proposed in this pull request

  • Adds forked Autorest as a submodule which will be used to generate cmdlets.
  • Updates custom commands in the group module to include custom header parameter as expected by the auto generated Client API class.
  • Updates pipelines to run generation using the forked AutoREST.PowerShell repo.
  • Updates the global directive file (readme.graph.md) to point to the AutoREST.PowerShell submodule.

Generated cmdlets were published to an internal feed. Using cmdlets obtained from that feed manual tests involving PATCH, POST and DELETE operations were done while raptor handled GET operations. Use this script to register the feed as a PS repository.

Sample cmdlet with optional custom header parameter
image

**Sample cmdlet extracting the correct media type from file inputs and setting the correct request content type. (Intitally all input file requests defaulted to application/octet-stream
image

Sample cmdlet with optional parameter for specifying the media/content type for file inputs

image

Sample raptor test run results for stable tests
NB This was locally run because the feed was registered locally.
image

@timayabi2020 timayabi2020 requested a review from a team as a code owner January 24, 2024 20:14
@calebkiage
Copy link
Contributor

Is it possible to use the same syntax as CLI for headers?

i.e. --headers AcceptLanguage=en

@sebastienlevert
Copy link

I agree that the semicolon is a weird character for this situation. I'd rather have an actual hashtable / PSObject. Easier to build dynamically if needs be.

Thoughts?

@timayabi2020
Copy link
Contributor Author

timayabi2020 commented Jan 25, 2024

@calebkiage @sebastienlevert yes it can change, however I preferred the approach of setting http headers the way it is normally done in a curl request. e.g curl -H "Content-Type: application/json" https://graph.com.........

@sebastienlevert
Copy link

I think we should be using the same approach that Invoke-WebRequest (and other similar Cmdlets) are using. https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-webrequest?view=powershell-7.4#-headers

Copy link
Member

@peombwa peombwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing on the most asked for feature! Please see my comment below on improvements to the UX of the parameter.

src/Groups/beta/custom/NewMgBetaGroupMember_Create.cs Outdated Show resolved Hide resolved
@timayabi2020 timayabi2020 requested a review from peombwa January 29, 2024 13:56
Copy link
Member

@peombwa peombwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@timayabi2020 timayabi2020 merged commit 162c5c8 into dev Jan 29, 2024
3 checks passed
@timayabi2020 timayabi2020 deleted the powershell-local-autorest branch January 29, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cmdlets should accept request headers
4 participants