KEMBAR78
adds kiota to dotnet openapi tool by baywet · Pull Request #41928 · dotnet/aspnetcore · GitHub
Skip to content

Conversation

@baywet
Copy link

@baywet baywet commented May 30, 2022

follow up of https://github.com/baywet/aspnetcore/pull/1 CC @dougbu

Questions left:

  • Should the dependencies be added by the api definition package or by this process (through the same mechanism that adds the ApiDescription package)?
  • How are the TypeScript dependencies supposed to be added since we're talking about a different package manager?
  • Would it be possible to add target namespace, target folder, and api client class name parameters to the tool?
  • Why is the Option for CodeGenerator not using a generic type to pass the enum and leave the parsing to system.command line?

Resulting CSProj (for now):

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.OpenApi.Kiota.ApiDescription.Client" Version="0.5.0-preview" />
  </ItemGroup>
  <ItemGroup>
    <OpenApiReference Include="Mail.yml" SourceUrl="https://raw.githubusercontent.com/microsoftgraph/msgraph-sdk-powershell/dev/openApiDocs/v1.0/Mail.yml" CodeGenerator="KiotaCSharp" />
  </ItemGroup>
</Project>

Testing steps

Create a local testing project with a nuget.config

<?xml version="1.0" encoding="utf-8"?>
<configuration>
	<packageSources>
		<add key="localnuget" value="file:///workspaces/localnuget" />
	</packageSources>
</configuration>

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

baywet added 4 commits May 27, 2022 19:14
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>
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label May 30, 2022
@dnfadmin
Copy link

dnfadmin commented May 30, 2022

CLA assistant check
All CLA requirements met.

@mkArtakMSFT mkArtakMSFT added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label May 31, 2022
<_Parameter3>NSwagCSharp;NSwagTypeScript</_Parameter3>
</AssemblyAttribute>
<AssemblyAttribute Include="Microsoft.DotNet.Openapi.Tools.Internal.OpenApiDependencyAttribute">
<_Parameter1>Microsoft.OpenApi.Kiota.ApiDescription.Client</_Parameter1>
Copy link
Contributor

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❔

Copy link
Author

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.

Copy link
Contributor

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.

Copy link
Author

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?

Copy link
Contributor

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❔

Copy link
Author

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

@baywet
Copy link
Author

baywet commented May 31, 2022

@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?
Why is the Option for CodeGenerator not using a generic type to pass the enum and leave the parsing to system.command line?

@dougbu
Copy link
Contributor

dougbu commented May 31, 2022

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.

Why is the Option for CodeGenerator not using a generic type to pass the enum and leave the parsing to system.command line?

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.

baywet added 2 commits June 1, 2022 13:50
Signed-off-by: GitHub <noreply@github.com>
…netcore into feature/openapi-kiota

Signed-off-by: GitHub <noreply@github.com>
@baywet
Copy link
Author

baywet commented Jun 1, 2022

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.

@baywet
Copy link
Author

baywet commented Jun 1, 2022

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
Other than that, I need to connect with Brady to learn more about the TypeScript dependencies.

@bradygaster
Copy link
Member

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 !

@baywet
Copy link
Author

baywet commented Jun 2, 2022

Update, I've made progress thanks to the help provided by Doug, and answer provided by Vijay

TypeScript

There are effectively two cases:

  • asp.net mvc app (no SPA), the TypeScript source file gets generated to the obj folder. It's expected that people will copy it/serve it so they can use it on the front end. NSwag only relies on dependencies that are already in the Visual Studio TypeScript targets. So here we might be at a dead end where the build will complain about missing dependencies and we don't have a way to add them (either manually or programmatically)
  • asp.net with SPA included. In this case a client SPA gets scaffolded with a package.json in ClientApp directory (or equivalent, denominated by SPARoot). In that case we try to run an npm install command, and if it fails, display a comprehensive message.

Dotnet

I 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.

MSBUILD : error MSB1025: An internal failure occurred while running MSBuild.
Microsoft.Build.Framework.InternalErrorException: MSB0001: Internal MSBuild Error: Unexpected CopyLocal flag.
at Microsoft.Build.Tasks.CopyLocalStateUtility.IsCopyLocal(CopyLocalState state)
at Microsoft.Build.Tasks.ResolveAssemblyReference.LogResults(ReferenceTable dependencyTable, List`1 idealAssemblyRemappings, List`1 idealAssemblyRemappingsIdentities, List`1 generalResolutionExceptions)
at Microsoft.Build.Tasks.ResolveAssemblyReference.Execute(FileExists fileExists, DirectoryExists directoryExists, GetDirectories getDirectories, GetAssemblyName getAssemblyName, GetAssemblyMetadata getAssemblyMetadata, GetLastWriteTime getLastWriteTime, GetAssemblyRuntimeVersion getRuntimeVersion, GetAssemblyPathInGac getAssemblyPathInGac, IsWinMDFile isWinMDFile, ReadMachineTypeFromPEHeader readMachineTypeFromPEHeader)
at Microsoft.Build.Tasks.ResolveAssemblyReference.Execute()
at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask)
at Microsoft.Build.BackEnd.TaskBuilder.InitializeAndExecuteTask(TaskLoggingContext taskLoggingContext, ItemBucket bucket, IDictionary`2 taskIdentityParameters, TaskHost taskHost, TaskExecutionMode howToExecuteTask)
at Microsoft.Build.BackEnd.TaskBuilder.ExecuteBucket(TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask, Dictionary`2 lookupHash)
at Microsoft.Build.BackEnd.TaskBuilder.ExecuteTask(TaskExecutionMode mode, Lookup lookup)
at Microsoft.Build.BackEnd.TaskBuilder.ExecuteTask(TargetLoggingContext loggingContext, BuildRequestEntry requestEntry, ITargetBuilderCallback targetBuilderCallback, ProjectTargetInstanceChild taskInstance, TaskExecutionMode mode, Lookup inferLookup, Lookup executeLookup, CancellationToken cancellationToken)
at Microsoft.Build.BackEnd.TargetEntry.ProcessBucket(ITaskBuilder taskBuilder, TargetLoggingContext targetLoggingContext, TaskExecutionMode mode, Lookup lookupForInference, Lookup lookupForExecution)
at Microsoft.Build.BackEnd.TargetEntry.ExecuteTarget(ITaskBuilder taskBuilder, BuildRequestEntry requestEntry, ProjectLoggingContext projectLoggingContext, CancellationToken cancellationToken)
at Microsoft.Build.BackEnd.TargetBuilder.ProcessTargetStack(ITaskBuilder taskBuilder)
at Microsoft.Build.BackEnd.TargetBuilder.BuildTargets(ProjectLoggingContext loggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, String[] targetNames, Lookup baseLookup, CancellationToken cancellationToken)
at Microsoft.Build.BackEnd.RequestBuilder.BuildProject()
at Microsoft.Build.BackEnd.RequestBuilder.BuildAndReport()
at Microsoft.Build.BackEnd.RequestBuilder.RequestThreadProc(Boolean setThreadParameters)

@baywet
Copy link
Author

baywet commented Jun 21, 2022

After chatting further with Doug, he requested a binlog for that error, here is the file for everybody else.
msbuild.zip

@captainsafia captainsafia removed the community-contribution Indicates that the PR has been added by a community member label Jan 23, 2023
@adityamandaleeka
Copy link
Member

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?

@captainsafia
Copy link
Member

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?

@baywet
Copy link
Author

baywet commented Jun 12, 2023

Thanks for the nudge here. Deferring to @darrelmiller who wanted me to keep things open until discussions pan out a while ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants