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

Bug - Latest Graph metadata changes causes Generation reports errors using #1668

Closed
nikithauc opened this issue Jun 27, 2022 · 8 comments
Closed
Assignees
Labels
fixed generator Issues or improvements relater to generation capabilities. type:bug A broken experience

Comments

@nikithauc
Copy link
Contributor

image

image

The latest changes microsoftgraph/msgraph-metadata@0e2b708 cause bugs in generation.

@nikithauc nikithauc added type:bug A broken experience generator Issues or improvements relater to generation capabilities. labels Jun 27, 2022
@zengin
Copy link
Contributor

zengin commented Jun 27, 2022

@rkodev, @fey101. It looks like the snippet generation errors are due to the last Open API update. The error is not specific to the snippet generation scenario.

@nikithauc
Copy link
Contributor Author

@zengin Should this issue be closed since the issue is traceable to the OpenAPI doc?

@zengin
Copy link
Contributor

zengin commented Jun 28, 2022

The issue is mitigated, but it will come back with tomorrow's scheduled generation run, unless I fix microsoftgraph/MSGraph-SDK-Code-Generator#782 beforehand to block releasing the OpenAPI doc.

Even then we need another issue where we fix the root cause parsing error. I don't know where that belongs at the moment as I am not sure if it is the document itself or the parsing library (cc: @andrueastman @irvinesunday @baywet). Only after root causing that, we should close this issue in favor of the underlying issue IMO.

@andrueastman
Copy link
Member

This looks to be an issue with the conversion library which looks to be generating incorrect openApi descriptions. microsoft/OpenAPI.NET.OData#243 has been created to follow up on this.

@baywet
Copy link
Member

baywet commented Jun 28, 2022

Hey everyone,
Thanks for jumping on the issue, I've been working to root cause that, and discovered a number of issues. Here is a recap of what I've found

  1. The OpenAPI.net validation rule is wrong and should look into anyOf/allOf/oneOf before triggering
  2. The conversion process is missing the odata type property, and should mark it as required as mandated by the spec
  3. The discriminator mapping should list additional ancestors
  4. The CSharp code writer don't disambiguate enough when namespaces conflict with type names
  5. kiota core generation engine duplicates some properties (this might be a side effect of the incorrect metadata
  6. kiota java weekly generation has been failing due to a fault in one of the generation scripts

Now, before Mustafa added the validation step (which I believe should be using hidi instead), the actual blocking step was the disambiguation one in CSharp, which I think we should prioritize. Kiota does log errors on invalid OpenAPI documents but doesn't fail. Of course we should also prioritize fixing the OpenAPI.net and the conversion lib (1, 2, 3) as it impacts more than just Kiota at this point and other tools/apps might choose to completely fail on invalid descriptions (devx API).

I hope this helps everyone see the bigger picture, don't hesitate if you have questions and/or comments!

@baywet baywet self-assigned this Jun 28, 2022
@baywet baywet added this to Kiota Jun 28, 2022
@baywet baywet moved this to Todo in Kiota Jun 28, 2022
@andrueastman
Copy link
Member

I think we can close this one for now as microsoft/OpenAPI.NET.OData#243 has been resolved and the errors are no longer reported on parsing the openApi document.

cc @baywet

@baywet
Copy link
Member

baywet commented Jul 18, 2022

Closing. Can you help make sure a release of hidi with the updated conversion library is out today so the generation pipeline tomorrow takes advantage of all the fixes please?

@baywet baywet closed this as completed Jul 18, 2022
Repository owner moved this from Todo to Done in Kiota Jul 18, 2022
@baywet baywet added the fixed label Jul 18, 2022
@andrueastman
Copy link
Member

Closing. Can you help make sure a release of hidi with the updated conversion library is out today so the generation pipeline tomorrow takes advantage of all the fixes please?

No worries. Will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed generator Issues or improvements relater to generation capabilities. type:bug A broken experience
Projects
Archived in project
Development

No branches or pull requests

4 participants