Skip to content

Conversation

@thomas11
Copy link
Contributor

@thomas11 thomas11 commented Feb 27, 2025

TODO

  • 4 modules have "composite" or empty versions
  • because we're reading the readme.md files from main or close to it, we're picking versions that are too new
  • not taking into account service groups
    • most modules have a single readme.md here: quota/resource-manager/readme.md
    • but service groups have one per service:
      • containerservice/resource-manager/Microsoft.ContainerService/aks/readme.md
      • containerservice/resource-manager/Microsoft.ContainerService/fleet/readme.md
    • they can have different API versions
    • some modules like workloads even have the op-level readme and service group readmes; not sure what that means
  • make this new way of getting default versions easily swappable with the previous one
  • allow manual overrides

@thomas11 thomas11 self-assigned this Feb 27, 2025
@thomas11 thomas11 changed the title Parse default version from a readme.md. Doesn't support composite packages. Read "official" default API versions from spec readmes Feb 27, 2025
@thomas11 thomas11 force-pushed the tkappler/default-versions-from-spec branch from e2f0158 to d7436dc Compare February 28, 2025 16:33
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@thomas11 thomas11 force-pushed the tkappler/default-versions-from-spec branch from d7436dc to b990681 Compare March 3, 2025 08:46
@codecov
Copy link

codecov bot commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 12.12121% with 116 lines in your changes missing coverage. Please review.

Project coverage is 57.06%. Comparing base (927be8f) to head (b990681).

Files with missing lines Patch % Lines
provider/pkg/resources/resources.go 1.72% 57 Missing ⚠️
provider/pkg/openapi/discover.go 2.22% 44 Missing ⚠️
provider/cmd/pulumi-gen-azure-native/main.go 0.00% 12 Missing ⚠️
provider/pkg/openapi/defaultApiVersions.go 82.35% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3996      +/-   ##
==========================================
- Coverage   57.52%   57.06%   -0.46%     
==========================================
  Files          82       83       +1     
  Lines       13096    13225     +129     
==========================================
+ Hits         7533     7547      +14     
- Misses       4987     5101     +114     
- Partials      576      577       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +47 to +50
if inYamlBlock && strings.HasPrefix(line, "tag: package-") {
version = strings.TrimSpace(strings.TrimPrefix(line, "tag: package-"))
break
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at some more readme files, I think we need to do at least one more step of processing to determine what the version selection is.

Where there's an initial code block such as:

``` yaml
title: ApiManagementClient
description: ApiManagement Client
openapi-type: arm
tag: package-preview-2024-06
```

The tag will correspond with other, conditional code blocks such as:

``` yaml $(tag) == 'package-preview-2024-06'
input-file:
  - Microsoft.ApiManagement/preview/2024-06-01-preview/apigateway.json
  - Microsoft.ApiManagement/preview/2024-06-01-preview/apimallpolicies.json
  - Microsoft.ApiManagement/preview/2024-06-01-preview/apimanagement.json
  - ...
```

The tag can be somewhat arbitrary, but the input-file list is the ultimate output we need.

Overall, I think the parsing approach for the data from a readme file is as follows (though we could possibly short-cut some of this, given we're not needing to support user inputs on the tag to use):

  1. Iterate through each yaml code block.
  2. Check its condition against the current collected state.
  3. Merge the block with the current parsed yaml state.

Worked example

With the two blocks defined above, after parsing the first block would result in a state of:

title: ApiManagementClient
description: ApiManagement Client
openapi-type: arm
tag: package-preview-2024-06

When evaluating the second block, the value of $(tag) would be package-preview-2024-06 and the condition evaluates to true. Therefore the state after parsing the second block is:

title: ApiManagementClient
description: ApiManagement Client
openapi-type: arm
tag: package-preview-2024-06
input-file:
  - Microsoft.ApiManagement/preview/2024-06-01-preview/apigateway.json
  - Microsoft.ApiManagement/preview/2024-06-01-preview/apimallpolicies.json
  - Microsoft.ApiManagement/preview/2024-06-01-preview/apimanagement.json
  - ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will probably need to turn the list of files back into a list of resources with version instead to produce a lockfile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Fix paths relative root to match existing reports.
- Generate diff against current v3 selection (from branch).
@EronWright
Copy link
Contributor

EronWright commented Mar 13, 2025

I was thinking that we would avoid duplication of their core parsing logic by having our tools consume the "code model" that is produced by autorest - the intermediate data format that autorest generates before doing language-specific code generation (across all languages supported by the Azure SDK). This is possible by running autorest with a certain flag to retain the code model, or by installing an autorest language plugin (that would target Pulumi schema).

Have you considered an approach like that?

Update: here's an example of that approach in action, in this case to generate the bicep schema files. What's missing here is fuller coverage of the resource functions, and perhaps more information about parentage and how to construct the full REST API path for a given resource.

https://github.com/Azure/bicep-types-az

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants