Test PowerShell integration with `Microsoft.OpenAPI.OData - v1.0.10-preview2` lib. #902

irvinesunday · 2022-03-07T17:37:21Z

The Microsoft.OpenAPI.OData - v1.0.10-preview2 OData - OpenAPI conversion lib. is out!
We need to test how this lib. integrates with PowerShell through DevX API via this test url: https://devxapitest.azurewebsites.net/ and bump up the lib version in this repo if successful.

The text was updated successfully, but these errors were encountered:

peombwa · 2022-03-09T00:58:47Z

Here are the results from the first test with PowerShell SDK:

All but three modules build after adding directives to fix circular references and broken commands

DeviceManagement.Functions fails because we are using content instead of schema in the parameter object for GET /deviceManagement/microsoft.graph.getRoleScopeTagsByIds(ids=@ids) operation.

AutoREST.PowerShell doesn't support parameter objects with content. In this case, there is no need for a content-type in a parameter. The ideal parameter object should be:

/deviceManagement/microsoft.graph.getRoleScopeTagsByIds(ids=@ids):
  get:
    tags:
      - deviceManagement.Functions
    summary: Invoke function getRoleScopeTagsByIds
    operationId: deviceManagement_getRoleScopeTagsByIds
    parameters:
      - name: ids
        in: query
        description: 'Usage: ids={ids}'
        required: true
        schema:
          type: array
          items:
            type: string
      ...

Sites and Files timeout after 8+ hours due to the number of paths and schemas. We will need to review the paths individually and remove those that are invalid.

Sites

v1.0

  ➜ hidi validate --openapi .\v1.0\Sites.yml
  Path Items: 715
  Operations: 1257
  Parameters: 7309
  Request Bodies: 388
  Responses: 1257
  Links: 1558
  Callbacks: 0
  Schemas: 11632

beta

  ➜ hidi validate --openapi .\beta\Sites.yml
  Path Items: 507
  Operations: 880
  Parameters: 4521
  Request Bodies: 282
  Responses: 880
  Links: 1069
  Callbacks: 0
  Schemas: 9820

Files

v1.0

  ➜ hidi validate --openapi .\v1.0\Files.yml
  Path Items: 546
  Operations: 1102
  Parameters: 4190
  Request Bodies: 384
  Responses: 1102
  Links: 979
  Callbacks: 0
  Schemas: 8018

beta

  ➜ hidi validate --openapi .\beta\Files.yml
  Path Items: 733
  Operations: 1409
  Parameters: 5435
  Request Bodies: 489
  Responses: 1409
  Links: 1640
  Callbacks: 0
  Schemas: 11174

peombwa · 2022-03-10T16:49:35Z

@irvinesunday, from my local test, the number of operations in Sites and Files can be substantially reduced by suppressing unwanted duplicate/invalid paths in the metadata (~1000 operations in v1.0 & beta), e.g.:

/sites/{site-id}/lists/{list-id}/activities is readonly, we shouldn't generate CUD operations.
/sites/{site-id}/lists/{list-id}/activities/{itemActivityOLD-id}* are all invalid. You can only list recent activities on a list, listItem, drive, & driveItem.
Plus, all the paths in Add Explode attribute to specific nav properties msgraph-metadata#105 with explode=false.

Are we in a position to use microsoftgraph/msgraph-metadata#105 (as a namespaced annotation) in 1.0.10 or should we fall back to containsTarget=false?

irvinesunday · 2022-03-14T08:30:26Z

Here are the results from the first test with PowerShell SDK:

All but three modules build after adding directives to fix circular references and broken commands

DeviceManagement.Functions fails because we are using content instead of schema in the parameter object for GET /deviceManagement/microsoft.graph.getRoleScopeTagsByIds(ids=@ids) operation.

AutoREST.PowerShell doesn't support parameter objects with content. In this case, there is no need for a content-type in a parameter. The ideal parameter object should be:

/deviceManagement/microsoft.graph.getRoleScopeTagsByIds(ids=@ids):
  get:
    tags:
      - deviceManagement.Functions
    summary: Invoke function getRoleScopeTagsByIds
    operationId: deviceManagement_getRoleScopeTagsByIds
    parameters:
      - name: ids
        in: query
        description: 'Usage: ids={ids}'
        required: true
        schema:
          type: array
          items:
            type: string
      ...

Sites and Files timeout after 8+ hours due to the number of paths and schemas. We will need to review the paths individually and remove those that are invalid.

Sites

v1.0

  ➜ hidi validate --openapi .\v1.0\Sites.yml
  Path Items: 715
  Operations: 1257
  Parameters: 7309
  Request Bodies: 388
  Responses: 1257
  Links: 1558
  Callbacks: 0
  Schemas: 11632

beta

  ➜ hidi validate --openapi .\beta\Sites.yml
  Path Items: 507
  Operations: 880
  Parameters: 4521
  Request Bodies: 282
  Responses: 880
  Links: 1069
  Callbacks: 0
  Schemas: 9820

Files

v1.0

  ➜ hidi validate --openapi .\v1.0\Files.yml
  Path Items: 546
  Operations: 1102
  Parameters: 4190
  Request Bodies: 384
  Responses: 1102
  Links: 979
  Callbacks: 0
  Schemas: 8018

beta

  ➜ hidi validate --openapi .\beta\Files.yml
  Path Items: 733
  Operations: 1409
  Parameters: 5435
  Request Bodies: 489
  Responses: 1409
  Links: 1640
  Callbacks: 0
  Schemas: 11174

The content in the parameter property for the DeviceManagement.Functions was added in this PR.
@baywet's initial schema proposal was similar to @peombwa's, as seen in this issue.

@darrelmiller is your initial proposal still valid in light of the above?

baywet · 2022-03-14T13:30:55Z

@irvinesunday the issue you linked refers to the return type conversion, not necessarily the parameters although the example is similar.

irvinesunday · 2022-03-14T14:26:29Z

@irvinesunday the issue you linked refers to the return type conversion, not necessarily the parameters although the example is similar.

Really?
Check out this fix:

I believe this is what added the content property in the parameter object

baywet · 2022-03-14T14:30:32Z

ah! I missing the content aspect during the PR review, my bad...

irvinesunday · 2022-03-14T14:33:07Z

ah! I missing the content aspect during the PR review, my bad...

It was suggested in @darrelmiller's proposal.

irvinesunday · 2022-03-14T16:13:49Z

@irvinesunday, from my local test, the number of operations in Sites and Files can be substantially reduced by suppressing unwanted duplicate/invalid paths in the metadata (~1000 operations in v1.0 & beta), e.g.:

/sites/{site-id}/lists/{list-id}/activities is readonly, we shouldn't generate CUD operations.

/sites/{site-id}/lists/{list-id}/activities/{itemActivityOLD-id}* are all invalid. You can only list recent activities on a list, listItem, drive, & driveItem.

Plus, all the paths in Add Explode attribute to specific nav properties msgraph-metadata#105 with explode=false.

Are we in a position to use microsoftgraph/msgraph-metadata#105 (as a namespaced annotation) in 1.0.10 or should we fall back to containsTarget=false?

@peombwa
We should fall back to ContainsTarget=false
I have updated the https://devxapitest.azurewebsites.net/ endpoint with your recommendations of the containments to be removed. Please try the Files and Sites modules again to validate whether this change might have improved things.

peombwa · 2022-03-14T20:25:08Z

@irvinesunday, has https://devxapitest.azurewebsites.net/ been updated? The diff yields no change in the OpenAPI docs (files and sites).

irvinesunday · 2022-03-15T11:58:59Z

Sorry @peombwa I forgot to update the updated metadata url in the https://devxapitest.azurewebsites.net/ app service. It should be good now.

peombwa · 2022-03-15T23:42:37Z

@irvinesunday, using the latest OpenAPI refresh, I'm able to generate Files and Sites successfully.

baywet · 2022-03-16T00:23:33Z

That's great news! @irvinesunday can you pr the conversion lib to ship 1.0.10 please? (release) the go generation based off this new version also fixed a lot of issues we were running into.

irvinesunday · 2022-03-16T13:21:35Z

That's great news! @irvinesunday can you pr the conversion lib to ship 1.0.10 please? (release) the go generation based off this new version also fixed a lot of issues we were running into.

There's still that issue with DeviceFunctions.Management parameter object content vs schema that needs to be resolved in the conversion lib.

darrelmiller · 2022-03-16T13:31:52Z

@irvinesunday The use of the content property indicates that the URL is serialized like this

/deviceManagement/microsoft.graph.getRoleScopeTagsByIds(ids=["1","2","3"])

with the schema property it says the URL looks like

/deviceManagement/microsoft.graph.getRoleScopeTagsByIds(ids=1,2,3)

If the later works with OData, then we can definitely change the output of the CSDL conversion. If it doesn't then I don't know how we solve this issue.

baywet · 2022-03-16T13:55:36Z

@irvinesunday I have created a milestone as well as an issue for the release https://github.com/microsoft/OpenAPI.NET.OData/milestone/6

Can you log an issue for the previous comment from Darrel and add it to the milestone please?
This way we have a clear line of sight on what's required to release that version, and how much progress we're making.

irvinesunday · 2022-03-16T16:05:46Z

@irvinesunday I have created a milestone as well as an issue for the release https://github.com/microsoft/OpenAPI.NET.OData/milestone/6

Can you log an issue for the previous comment from Darrel and add it to the milestone please? This way we have a clear line of sight on what's required to release that version, and how much progress we're making.

@baywet I have created an issue in DevX API (I believe this is a PowerShell-specific fix) and linked it to the milestone issue you've created.

irvinesunday · 2022-03-16T16:07:44Z

@peombwa try testing the DeviceManagement.Functions again with the updated https://devxapitest.azurewebsites.net/. I have patched a fix to address the content --> schema issue.

peombwa · 2022-03-16T17:21:51Z

@peombwa try testing the DeviceManagement.Functions again with the updated https://devxapitest.azurewebsites.net/. I have patched a fix to address the content --> schema issue.

@irvinesunday, the change to getRoleScopeTagsByIds looks good. However, we are now making every string parameter of a function an array of strings e.g., /deviceManagement/templates/{deviceManagementTemplate-id}/microsoft.graph.compare(templateId=''{templateId}'):

We should only generate schema parameters of array of strings when the parameter type is Collection(Edm.String) in the CSDL.

irvinesunday · 2022-03-16T22:47:20Z

/deviceManagement/templates/{deviceManagementTemplate-id}/microsoft.graph.compare(templateId=''{templateId}')

Thanks for the rapid bug bash @peombwa 😄 I have fixed that issue.

peombwa · 2022-03-17T15:59:27Z

Thanks for the quick turnaround. I've rebuilt the entire SDK, and everything looks good on PS's end. I'd say we are good to go with 1.0.10.

peombwa · 2022-04-05T23:16:42Z

@irvinesunday, I've just encountered #928 in this week's refresh. It's blocking PS from using Microsoft.OpenAPI.OData v1.0.10.

irvinesunday · 2022-04-06T09:40:42Z

Thanks for reporting this @peombwa. In an attempt to align our convert settings with Hidi, I inadvertently included this convert setting: DeclarePathParametersOnPathItem = true which causes this. This PR fixes that.

bartvermeersch · 2022-06-07T19:54:40Z

What is the status of this issue? Is it still blocking the availability/presence of the PS remove-* commandlets (Missing $ref paths for DELETE operations)

irvinesunday added the dependencies Pull requests that update a dependency file label Mar 7, 2022

irvinesunday assigned peombwa Mar 7, 2022

irvinesunday self-assigned this Mar 7, 2022

This was referenced Mar 11, 2022

Missing $ref paths for DELETE operations #567

Closed

Missing cmdlet: Remove-MgGroupMember microsoftgraph/msgraph-sdk-powershell#452

Closed

irvinesunday mentioned this issue Mar 16, 2022

Remove further containments and indexability microsoftgraph/msgraph-metadata#130

Merged

irvinesunday mentioned this issue Mar 16, 2022

Resolve structured and collection-valued function parameters for PowerShell style #911

Closed

baywet mentioned this issue Mar 17, 2022

Resolves structured and collection-valued function parameters for PowerShell style #913

Merged

peombwa mentioned this issue Mar 18, 2022

Add Support for Parameter Objects with Content Property Azure/autorest#4480

Open

irvinesunday mentioned this issue Mar 25, 2022

Fixes paths for functions with structured or collection-valued parameters microsoft/OpenAPI.NET.OData#203

Merged

peombwa mentioned this issue Apr 5, 2022

Missing key segments in the parameter object in the latest OpenAPI refresh. #928

Closed

peombwa mentioned this issue Apr 6, 2022

Cryptic Error Message when generating module Azure/autorest#4503

Closed

peombwa mentioned this issue Jun 10, 2022

Refresh OpenAPI Docs microsoftgraph/msgraph-sdk-powershell#1315

Merged

peombwa closed this as completed in microsoftgraph/msgraph-sdk-powershell#1315 Jun 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test PowerShell integration with `Microsoft.OpenAPI.OData - v1.0.10-preview2` lib. #902

Test PowerShell integration with `Microsoft.OpenAPI.OData - v1.0.10-preview2` lib. #902

irvinesunday commented Mar 7, 2022

peombwa commented Mar 9, 2022 •

edited

Loading

peombwa commented Mar 10, 2022

irvinesunday commented Mar 14, 2022

baywet commented Mar 14, 2022

irvinesunday commented Mar 14, 2022

baywet commented Mar 14, 2022

irvinesunday commented Mar 14, 2022 •

edited

Loading

irvinesunday commented Mar 14, 2022

peombwa commented Mar 14, 2022 •

edited

Loading

irvinesunday commented Mar 15, 2022

peombwa commented Mar 15, 2022

baywet commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

darrelmiller commented Mar 16, 2022

baywet commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

peombwa commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

peombwa commented Mar 17, 2022

peombwa commented Apr 5, 2022

irvinesunday commented Apr 6, 2022

bartvermeersch commented Jun 7, 2022

Test PowerShell integration with Microsoft.OpenAPI.OData - v1.0.10-preview2 lib. #902

Test PowerShell integration with Microsoft.OpenAPI.OData - v1.0.10-preview2 lib. #902

Comments

irvinesunday commented Mar 7, 2022

peombwa commented Mar 9, 2022 • edited Loading

peombwa commented Mar 10, 2022

irvinesunday commented Mar 14, 2022

baywet commented Mar 14, 2022

irvinesunday commented Mar 14, 2022

baywet commented Mar 14, 2022

irvinesunday commented Mar 14, 2022 • edited Loading

irvinesunday commented Mar 14, 2022

peombwa commented Mar 14, 2022 • edited Loading

irvinesunday commented Mar 15, 2022

peombwa commented Mar 15, 2022

baywet commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

darrelmiller commented Mar 16, 2022

baywet commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

peombwa commented Mar 16, 2022

irvinesunday commented Mar 16, 2022

peombwa commented Mar 17, 2022

peombwa commented Apr 5, 2022

irvinesunday commented Apr 6, 2022

bartvermeersch commented Jun 7, 2022

Test PowerShell integration with `Microsoft.OpenAPI.OData - v1.0.10-preview2` lib. #902

Test PowerShell integration with `Microsoft.OpenAPI.OData - v1.0.10-preview2` lib. #902

peombwa commented Mar 9, 2022 •

edited

Loading

irvinesunday commented Mar 14, 2022 •

edited

Loading

peombwa commented Mar 14, 2022 •

edited

Loading