-
Notifications
You must be signed in to change notification settings - Fork 10.5k
adds kiota to dotnet openapi tool #41928
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
Conversation
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
| <_Parameter3>NSwagCSharp;NSwagTypeScript</_Parameter3> | ||
| </AssemblyAttribute> | ||
| <AssemblyAttribute Include="Microsoft.DotNet.Openapi.Tools.Internal.OpenApiDependencyAttribute"> | ||
| <_Parameter1>Microsoft.OpenApi.Kiota.ApiDescription.Client</_Parameter1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referencing only this package, will updated projects using KiotaCSharp or KiotaTypeScript be able to successfully use a generated client❔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at the moment, they'll need at least Microsoft.Kiota.Abstractions for dotnet projects and @microsoft/kiota-abstractions for typescript projects to compile.
They'll also need Microsoft.Kiota.Http.HttpClientLibrary/@microsoft/kiota-http-fetch or an alternative implementation to execute the requests
They'll also need Microsoft.Kiota.Serialization.Json / @microsoft/kiota-serialization-json or an alternative implementation to handle serialization/deserialization.
And finally, they might need Microsoft.Kiota.Authentication.Azure/@microsoft/kiota-authentication-azure or an alternative if their API requires authentication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🆗 then this PR is not complete as-is. When using the NSwag generator options, the generated client is immediately useful and this should be the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would be the recommended approach from your point of view? add those dependencies to the APIDescription package?
Or add those dependencies to the target through the same mechanism we're adding the API description package?
I prefer the later as it provides more control to users over which dependencies and versions dependencies they bring to their project should they want to change things.
Also, how are the JS/TS/npm packages added? are they being wrapped in a nuget package? or is there a possibility to add a reference to an npm package? if so how?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add those dependencies to the target through the same mechanism we're adding the API description package?
This would be my preference.
how are the JS/TS/npm packages added?
I'm not sure what JS/TS/npm dependencies an NSwag client requires or how those dependencies are handled. For all I know, the generated client works standalone. @bradygaster do you have more information❔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added the dependencies for dotnet. waiting on @bradygaster 's input for the typescript ones
|
@dougbu outside of the dependencies questions, I also had those two questions for you, thanks for the help! Would it be possible to add target namespace, target folder, and api client class name parameters to the tool? |
That sounds like additional customization users could do based on the incredibly poor "documentation" in https://github.com/dotnet/aspnetcore/blob/main/src/Tools/Extensions.ApiDescription.Client/src/build/Microsoft.Extensions.ApiDescription.Client.props. I would recommend adjusting the defaults to your preferences in the Microsoft.OpenApi.Kiota.ApiDescription.Client package and adding a bit more information for users over complicating the tool w/ lots of new options. Editing the updated project file isn't that big a deal.
I don't remember the full history. @captainsafia made the most recent changes to the tool, @ryanbrandenburg wrote it, and @BillHiebert did the VS side of things and designed the service that provides some of the versioning. @bradygaster may also have context. |
Signed-off-by: GitHub <noreply@github.com>
…netcore into feature/openapi-kiota Signed-off-by: GitHub <noreply@github.com>
|
thanks for the additional information. I'm not able to understand how this file works and what pieces I need just from reading it. Schedule a meeting today with you to go over that together. |
|
Update for everyone watching this PR, during our connect with Doug we've made some updates to the kiota packages that I still need to test and publish. microsoft/kiota#1604 |
|
i'm here, watching. :) we touched base today but I'm slim on the NPM knowledge. Is this something with which @vijayrkn could help? Maybe @phil-allen-msft could guide us in the right direction? Thanks for pushing on this @baywet ! |
|
Update, I've made progress thanks to the help provided by Doug, and answer provided by Vijay TypeScriptThere are effectively two cases:
DotnetI got to a place where the packaging and the command seems to be structured properly, if I execute the command manually in the terminal, it works. However when I try to build the project (dotnet build) I get the following error message. I'm not sure where to go from there. |
|
After chatting further with Doug, he requested a binlog for that error, here is the file for everybody else. |
|
I just noticed this PR. Looks like it's been silent for nearly a year... is this still being worked on or should it be closed at this point? |
|
I think it should be fine to close this. We'll likely want to put in some design work into how the client gen code can support new providers. @baywet Thoughts? |
|
Thanks for the nudge here. Deferring to @darrelmiller who wanted me to keep things open until discussions pan out a while ago. |
follow up of https://github.com/baywet/aspnetcore/pull/1 CC @dougbu
Questions left:
Resulting CSProj (for now):
Testing steps
Create a local testing project with a nuget.config
Add Kiota and Kiota ApiDescription to the local source
dotnet pack for kiota and kiota api ddescription, copy in the previous local source folder
Pack the tool and run it
cd src/Tools/Microsoft.dotnet-openapi/src dotnet pack dotnet tool install --local Microsoft.dotnet-openapi --add-source /workspaces/aspnetcore/artifacts/packages/Debug/Shipping/ --version 7.0.0-dev dotnet tool run dotnet-openapi -- add url https://raw.githubusercontent.com/microsoftgraph/msgraph-sdk-powershell/dev/openApiDocs/v1.0/Mail.yml -c KiotaCSharp -p /workspaces/dummy/dummy.csproj dotnet tool uninstall --local Microsoft.dotnet-openapi