-
Notifications
You must be signed in to change notification settings - Fork 218
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
CLI generation does not handle DateTime path parameters #4615
Comments
Hi @baywet, I want to debug and work on this issue, can I get this issue assigned and could you please give me command to reproduce this bug, thanks |
@rohitkrsoni Thanks for volunteering. I'm guessing the problem is located here where the string needs to be parsed before being assigned.
|
Hi @baywet, I am going to pause my investigation on this bug as the other issue I was working on is progressing but here are my investigation so far and I am happy to work on it after that. The build is failing because our generator is trying to assign a string to System.DateTimeOffSet Here q.QueryParameters.StartDateTime is of type DateTimeOffset And startDateTime variable is of type string as:
Now, this is happening because: To create the API Client Class and it's properties (query parameters), GetPrimitiveType() method is used to get the type of the parameter. kiota/src/Kiota.Builder/KiotaBuilder.cs Line 1070 in 47b1505
Based on the type and format of the parameter, the type is returned kiota/src/Kiota.Builder/KiotaBuilder.cs Lines 1085 to 1107 in 47b1505
But when it comes to the value of the option which our generator is trying to set for the query parameter, things are different.
And the parametersList is passed from the method WriteExecutableCommand kiota/src/Kiota.Builder/Writers/CLI/CliCodeMethodWriter.cs Lines 73 to 79 in 47b1505
And generatorMethod.PathQueryAndHeaderParameters is set in KiotaBuilder.cs kiota/src/Kiota.Builder/KiotaBuilder.cs Lines 1382 to 1395 in 47b1505
In GetCodeParameterFromApiParameter(), the Type is being set by GetQueryParameterType(x.Schema) kiota/src/Kiota.Builder/KiotaBuilder.cs Lines 1359 to 1367 in 47b1505
And lastly, GetQueryParameterType(x.Schema) is setting the name of the type of the parameter by just using the type of the parameter and not using the format as it was done by the GetPrimitiveType() kiota/src/Kiota.Builder/KiotaBuilder.cs Lines 2340 to 2346 in 47b1505
Hence the type is always string for the parameter of type 'string' and format 'date-time' And it should give error for all the parameter types in which the type is "string" and format is something else which is creating a different type of Query Parameter in the ClientAPIClass kiota/src/Kiota.Builder/KiotaBuilder.cs Lines 1088 to 1095 in 47b1505
I tried one for "date" format and got the similar error. Hope I am able to explain the issue well, and I feel like I have added lots of information (maybe even those which are straight forward) but I was not sure how to put it all together. I have not looked for the solution yet but I am happy to work on it after the completion of the other issue and your insights will be very helpful. PS: Sorry for the previous notification, it was a mis-click. Thanks |
Thanks for the detailed investigation. |
Thanks, and yes that did work, I checked manually kiota/src/Kiota.Builder/KiotaBuilder.cs Lines 2340 to 2346 in 47b1505
And this will enable the generator to create options of the specific type rather than just string in I am just not sure about the effect of the change on the project! |
Thanks for the additional information. For the option, the type is hardcoded here I think this is mostly because the system.commandline library doesn't support a lot of types when parsing without extensive configuration, but @calebkiage could confirm this. I don't think you need to change anything in the main builder at this point. Here you should be able to navigate from the generator method, to the query parameters class, and then get the query parameter property which has the correct type if that makes sense? |
The System.CommandLine library supports for dates as option and argument types. So the code |
Thanks for the clarification. @calebkiage |
yes, the more work we can delegate to the dependencies, the better. |
…yParameterType()
Hi, here is the PR for the change, Please review it, thanks |
What are you generating using Kiota, clients or plugins?
API Client/SDK
In what context or format are you using Kiota?
Source Build
Client Library/SDK Language
CLI
Describe the bug
Microsoft Graph beta CLI generation is failing with the following error:
(x12)
Expected behavior
CLI to handle DateTime path parameters properly.
How to reproduce
Open API description file
Kiota Version
1.14.0
Latest Kiota version known to work for scenario above?(Not required)
No response
Known Workarounds
No response
Configuration
No response
Debug output
https://microsoftgraph.visualstudio.com/Graph%20Developer%20Experiences/_build/results?buildId=147067&view=logs&j=e7ad2393-6f47-5a6c-43fe-f034bda9890e&t=ec6c7d4d-190c-5a0e-65ac-93472f7d2fef
Click to expand log
```The text was updated successfully, but these errors were encountered: