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

Issues with writing the Lock file on the WASM runtime #2152

Closed
andreaTP opened this issue Jan 13, 2023 · 21 comments
Closed

Issues with writing the Lock file on the WASM runtime #2152

andreaTP opened this issue Jan 13, 2023 · 21 comments
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. help wanted Issue caused by core project dependency modules or library
Milestone

Comments

@andreaTP
Copy link
Contributor

I'm unable to reproduce locally, but this does happen with the output of a CI:

System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|143_0()
   at Kiota.Builder.Lock.LockManagementService.WriteLockFileInternalAsync(String , KiotaLock , CancellationToken )
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|143_0()
   at Kiota.Builder.Lock.LockManagementService.WriteLockFileInternalAsync(String , KiotaLock , CancellationToken )
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|143_0()
   at Kiota.Builder.KiotaBuilder.UpdateLockFile(CancellationToken )
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|143_0()
   at Kiota.Builder.KiotaBuilder.GenerateClientAsync(CancellationToken )
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|143_0()
   at KiotaClientGen.Generate(String , String , String , String )
Error: The deserialization constructor for type 'Kiota.Builder.Lock.KiotaLock' contains parameters with null names. This might happen because the parameter names have been trimmed by the linker. Consider using the source generated serializer instead.
@andreaTP
Copy link
Contributor Author

I "think" that the issue appeared when compiling with dotnet publish instead of dotnet build, but I will confirm.

@baywet
Copy link
Member

baywet commented Feb 6, 2023

@andreaTP is this still an issue or can it be closed at this time?

@baywet baywet added this to the Kiota post-GA milestone Feb 6, 2023
@baywet
Copy link
Member

baywet commented Feb 6, 2023

related #2149

@andreaTP
Copy link
Contributor Author

andreaTP commented Feb 7, 2023

@baywet I do believe that this is still an issue.

Let me give you some context, on the Maven Plugin for Kiota I'm currently simply downloading and running the Kiota CLI as an external process.
This works ok, but, a better level of integration can be achieved by running Kiota on WASM directly from Java(using something like the wasmtime or wasmer bindings), this approach can be re-used, eventually, in other languages/runtimes.
To achieve this goal we would need to compile Kiota for wasi-wasm (as opposed to browser-wasm).

Unfortunately, looking at this discussion it looks unlikely to have support for File System operations any time soon from .NET.

An option to support this use case would be to pass the utilities that are performing IO and system calls through interfaces to Kiota.Builder so that they can be implemented "externally" on different runtimes.
This comes with the downside of possible additional complexity and decoupling of the current codebase.

@baywet
Copy link
Member

baywet commented Feb 7, 2023

Thanks for the additional context here. It's strange those APIs don't work in your context, because it's exactly what we're doing with kiota web but maybe this is one of the subtle differences between blazor wasm and blazor web (I suspect the latter, being used by kiota web, adds additional layers under the hood)

@andreaTP
Copy link
Contributor Author

andreaTP commented Feb 7, 2023

The root issue here is the lack of a File System API when targeting wasi-wasm as opposed to browser-wasm(the latter is the one used in Blazor Kiota.Web and in my integration), unfortunately, WASI is not stable enough at this point in time and the FS semantics are somewhat dissimilar to a "real file system".

That said if I(ever) get the time to try to solve this, would you accept a PR doing the heavy lifting to decouple Kiota.Builder from using directly FileSystem and HttpClient (basically all of the interactions with the host system) APIs?
Or do you think that this is going to introduce too much complexity in the current codebase?
Additionally, do you have alternative ideas on how to overcome this limitation for the time being?

@baywet
Copy link
Member

baywet commented Feb 8, 2023

I'm on the fence for this aspect. First off, it's strange that wasi-wasm doesn't have file and http since it has multithreading and browser-wasm doesn't have it. My understanding was that wasi-wasm was leading the charge in terms of innovation and implementation and browser-wasm followed closely behind.

If we were to make these changes let's try to dive into the details

File

There are a couple of places where file APIs are being used:

  • reading the description (when sourcing locally)
  • reading and writing descriptions to/from the cache (when sourcing from the web)
  • reading and writing lock files
  • writing generated code
  • (and I hope I'm not forgetting anything)

It should be fairly easy to identify all places and design interfaces to add levels or indirections, the question remains, what is the API surface of kiota builder going to look like?

Http

There are a couple of places where http client is being used:

  • searching in APIs.guru
  • searching in github (that's a kiota client, and this one will be harder to indirect further unless we use the request adapter interface)
  • getting the api description (for the web)
  • (in the future) checking the local version of kiota is up to date
  • (and I hope I'm not forgetting anything)

The API surface is already requesting the http client to avoid the classical port exhaustion issue in dotnet for long lived processes.

@andreaTP
Copy link
Contributor Author

andreaTP commented Feb 8, 2023

Thanks for reverting back!

it's strange that wasi-wasm doesn't have file and http since it has multithreading and browser-wasm doesn't have it

this is not actually "strange" as browser-wasm has a longer and more solid history, on the other side, WASI is not fully specified and things are evolving and changing pretty fast there.
(I'm at the moment at a conference with my friend @evacchi who is full-time working on WAZero 🙂 and I'm confirming that information)

File

As far as I understand it, the WASI FS API is closer in semantics to a "sandboxed FS, hence it makes for a good in-browser experience but it doesn't have a semantic matching to a real File System implementation (explained in this discussion

what is the API surface of kiota builder going to look like?

this would be the starting point to prototype how changes will impact the codebase.

As I already mentioned this issue has been opened mostly for tracking purposes and for gathering better context but is definitely not at the top of my current priorities.

@baywet
Copy link
Member

baywet commented Feb 8, 2023

I've seen some pictures of the Red Hat summit, it looks pretty cool! Maybe next year we can present a joint session about Kiota and our collaboration?

Not a top priority on our side as well at the moment, but I appreciate the open discussion here. One aspect is it'd represent a major breaking change in the API surface and would mandate a major version bump of the Kiota builder lib. I don't believe it's a big deal, but something to keep in mind.

I guess that at this point the next step would be to design the interfaces and inventory the impact.

@baywet baywet added help wanted Issue caused by core project dependency modules or library enhancement New feature or request generator Issues or improvements relater to generation capabilities. and removed Needs: Attention 👋 question labels Feb 8, 2023
@baywet baywet added this to Kiota Feb 8, 2023
@github-project-automation github-project-automation bot moved this to Todo in Kiota Feb 8, 2023
@andreaTP
Copy link
Contributor Author

andreaTP commented Feb 8, 2023

I've seen some pictures of the Red Hat summit, it looks pretty cool!

This is actually JFokus, we had a session about something very different, but is an amazing conference(this year about 2000 ppl joining the event!) in the Java landscape.

Maybe next year we can present a joint session about Kiota and our collaboration?

I'll be honored to contribute to spreading this amazing project with you @baywet and the collaboration that is happening.
Let's make it happen! 🚀

I appreciate the open discussion here

Amazing interaction as always 🙏

You are correct about the fact that it's going to be a breaking change and we would need to carefully handle the situation.

I guess that at this point the next step would be to design the interfaces and inventory the impact.

Agreed, but keeping it low in priorities as things might get easier over time given that both WASI and the wasi-wasm compilation target for dotnet are evolving.

@baywet
Copy link
Member

baywet commented Feb 15, 2023

I've been poking around that space for the last couple of hours.
We're having a hack week and I'm trying to build a VS code extension for kiota. VS code runs on Electron which ships with a specific version of node (14 at time of writing).

My take away is that even using a wasmconsole project, which is supposed to support node, the dotnet.js still has dependencies on browser functions (namely window, and the way it requires submodules). It does feel like all that space is super edgy at the moment and the team didn't have node/interoperability with other runtimes in mind when they started out.

Browsers seem to be working great as we've demonstrated for kiota web and API curio.

In conclusion I believe we should let this tech grow before investing too much in wasm portability for kiota and maintaining a npm package. That's unless I missed something and additional evidence comes along.

@baywet
Copy link
Member

baywet commented Feb 16, 2023

another idea I'm currently exploring is using JSON-RPC. Effectively the vs code integration, spawns the process, interops with it using JSON RPC on stdin/out everytime it needs kiota. works like a charm. It might be a good alternative for your project as well. And we might want to consider maintaining an "official" kiota json RPC server.

@andreaTP
Copy link
Contributor Author

Thanks for sharing the process @baywet , in the Maven plugin, I'm already spawning the kiota cli as a separate process, in the context of this issue I was looking for a tighter integration/embedding.

But, an interesting homework might be to support machine-readable output (e.g. json/yaml) under a -o flag (like kubectl and similar).

@baywet
Copy link
Member

baywet commented Feb 17, 2023

If you want to have visibility over what I'm doing, check the feature/vscode-magic branch. Ignore the wasm part

@andreaTP
Copy link
Contributor Author

Update 21/07/2023

TLDR

Is it possible: YES! Can we do it today: not yet 😢

Motivation

I found this repository: https://github.com/lambdageek/sample-dotnet-wasi-vscode and it should be very much aligned with what @baywet attempted to do with the Kiota VS-Code extension.
I keep being interested in this subject as being able to integrate Kiota as a WASM application is going to have much fewer downsides than running as an external process(my short-term goal is to use it from a Maven Plugin).

Goal

Verify the current status of compiling Kiota to wasi-wasm target and evaluate the gap.

Experiment

Here you can find a super-hacked version of Kiota that actually runs on a WASI target(tested with wasmtime and wasmer):
https://github.com/microsoft/kiota/compare/main...andreaTP:wasi-attempt?expand=1
You would need those prerequisites to compile and run things locally.
Run, in the src/wasm-embedded-kiota directory with: wasmtime ./bin/Debug/net8.0/wasi-wasm/AppBundle/wasm-embedded-kiota.wasm 2> client.zip, the zip output is emitted on stderr and all the input parameters are hardcoded ATM as I haven't found a better way for doing it( 😢 ) at this stage.

Take-aways

A few useful considerations from the experiment:

  • Virtual File System: according to this issue we are not going to have a full vfs on WASI, in this POC I used Zio to bridge the gap. I noticed that Zio artifacts are not signed and I don't know if this is a blocker or not in its adoption, but, the abstraction is pretty cool, and having the possibility to inject the VFS from the caller is going to be necessary if there is not out-of-the-box support from the runtime.
  • HttpClient: this has been one of the major issues again, even the Microsoft.OpenAPI package, directly depends on the standard implementation of it. Being able to inject it pervasively is probably necessary in the longer term.
  • System.Security.Cryptography is not supported at all, and this causes failures when computing hashes. I can't find any relevant issue in the tracking issue. It would be good to know what's the future plan to decide if we should setup DI for those operations.
  • Async & Parallel.ForEach I faced a number of issues when attempting to use concurrency primitives(even a simple Task.Run(() => seems to be failing consistently), I ended up re-writing sync versions of the functions for the seek of speed. Somehow intertwined with the discussion about idempotency, would it be interesting to have a "fully-sync" version of Kiota? Or should we open more specific issues in the dotnet/runtime repo?

Next

I'm going on PTO tomorrow(so I might be slower in replies), but I can eventually work on those things.
I think that we can start to ease the transition with simple changes that can be evaluated:

  • setup proper DI for calculating Hash codes looks like a low-hanging fruit
  • evaluate using a VFS like Zio (or another incarnation of the same concept)
  • have the option to run Kiota locally with the network disabled, avoiding all of the invocations(even ctor) of HttpClient
  • ...

Thanks

This spike could not have been done without the demo repositories of @lambdageek and @SteveSandersonMS , thanks a lot for the hard work! ❤️

@baywet
Copy link
Member

baywet commented Jul 21, 2023

Thanks for sharing your learnings with us. On our side this has become less of a priority because:

  • We sunset app.kiota.dev last week
  • Graph Explorer integration will rely on deep links with the VS Code extension

@andreaTP
Copy link
Contributor Author

Thanks for the feedback @baywet !
A couple of follow-up questions:

We sunset app.kiota.dev last week

Does this have any implications for the project? E.g. is the Blazor app going to be still tested or should I set up alternative infrastructure for the Apicurio integration? Or do you have any additional recommendations?

Graph Explorer integration will rely on deep links with the VS Code extension

That's cool and I can't wait to see a demo 🙂

Regarding the WASI integration itself, would you be up to discussing possible steps forwards and eventually accepting PRs or is it a closed effort at all?

@baywet
Copy link
Member

baywet commented Jul 21, 2023

Does this have any implications for the project?

Repo has already been cleared of any "web" aspects (blazor project, playwright, etc...)

would you be up to discussing possible steps forwards and eventually accepting PRs

that'd really depend on the scope. If we're talking about minor changes that don't impact the CLI experience to facilitate the wasm aspects, yes. If we're talking about major refactor efforts, or things that are detrimental to the CLI experience, we'll probably need to see a strong justification.

@andreaTP
Copy link
Contributor Author

Thanks for the clarification, I'll get back when I have a more consistent plan in mind and/or when technology catch up 🙂

@lambdageek
Copy link
Member

would it be interesting to have a "fully-sync" version of Kiota? Or should we open more specific issues in the dotnet/runtime repo?

Please open issues for the runtime repo. While async doesn't really work on the wasi-experimental workload right now, it's definitely not a status quo we're happy with. More customer issues would help us prioritize the work.

That said, right now the workaround is, sadly, to do everything in a fully sync way. I am currently exploring that with my WIP MSBuild Log Viewer VS Code extension (https://github.com/lambdageek/MSBuildStructuredLog/tree/wasi-single-thread - send me an email inside MS, I'll share a brief demo video from last week) building on VS Code's recent WASI host work (https://code.visualstudio.com/blogs/2023/06/05/vscode-wasm-wasi). Interestingly, it's easier if you're starting from a legacy codebase that hadn't bought into async.

@baywet
Copy link
Member

baywet commented Aug 10, 2023

Thanks for reaching out. We sunset our webapp and we setup a JSON RPC server for other integrations (outside of the browser).

One of the reasons for that was the performance aspect. Another was the lack of some APIs (file access, http client, etc...) That would have required significant refactoring.

Closing.

@baywet baywet closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Kiota Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request generator Issues or improvements relater to generation capabilities. help wanted Issue caused by core project dependency modules or library
Projects
Archived in project
Development

No branches or pull requests

3 participants