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

Resolve remaining path conflicts in v3 #3855

Merged
merged 9 commits into from
Jan 23, 2025

Conversation

thomas11
Copy link
Contributor

@thomas11 thomas11 commented Jan 15, 2025

Resolves #2495.

The first two commits bring the conflicts down to one or two, depending on how you count.

{
  "MachineLearningServices": {
    "ConnectionRaiBlocklist": {
      "/subscriptions/{}/resourcegroups/{}/providers/microsoft.machinelearningservices/workspaces/{}/connections/{}/raiblocklists/{}": [
        "2024-07-01-preview",
        "2024-10-01-preview",
        "default"
      ],
      "/subscriptions/{}/resourcegroups/{}/providers/microsoft.machinelearningservices/workspaces/{}/connections/{}/raiblocklists/{}/raiblocklistitems/{}": [
        "2024-04-01-preview"
      ]
    },
    "ConnectionRaiBlocklistItem": {
      "/subscriptions/{}/resourcegroups/{}/providers/microsoft.machinelearningservices/workspaces/{}/connections/{}/raiblocklists/{}": [
        "2024-04-01-preview"
      ],
      "/subscriptions/{}/resourcegroups/{}/providers/microsoft.machinelearningservices/workspaces/{}/connections/{}/raiblocklists/{}/raiblocklistitems/{}": [
        "2024-07-01-preview",
        "2024-10-01-preview",
        "default"
      ]
    }
  }
}

TODO

  • ConnectionRaiBlocklist
    They made a mistake in 2024-0-01-preview, which is the v2 default version, in that they swapped the operationId’s of two resources.

    • GET for /raiblocklists/{} is ConnectionRaiBlocklistItem_Get
    • GET for /raiblocklists/{}/raiblocklistitems/{} is ConnectionRaiBlocklist_Get
  • Aliases

@thomas11 thomas11 requested review from danielrbradley, EronWright and a team January 15, 2025 06:37
Copy link

Does the PR have any schema changes?

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

Copy link

codecov bot commented Jan 15, 2025

Codecov Report

Attention: Patch coverage is 40.20619% with 58 lines in your changes missing coverage. Please review.

Project coverage is 57.37%. Comparing base (ee1cc82) to head (a372db5).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
provider/pkg/resources/resources.go 44.31% 34 Missing and 15 partials ⚠️
provider/pkg/versioning/gen.go 0.00% 5 Missing ⚠️
provider/pkg/gen/types.go 0.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3855      +/-   ##
==========================================
+ Coverage   57.26%   57.37%   +0.11%     
==========================================
  Files          79       79              
  Lines       12469    12456      -13     
==========================================
+ Hits         7140     7147       +7     
+ Misses       4799     4766      -33     
- Partials      530      543      +13     

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

Copy link
Member

@danielrbradley danielrbradley left a comment

Choose a reason for hiding this comment

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

Overall looks good. Might be worth a quick sync call to make sure I understand all the moving parts.

versions/v2-lock.json Outdated Show resolved Hide resolved
@thomas11 thomas11 marked this pull request as ready for review January 23, 2025 14:30
@thomas11 thomas11 force-pushed the tkappler/v3-resolve-path-conflicts branch from 5dd2f52 to a372db5 Compare January 23, 2025 15:20
Copy link
Member

@danielrbradley danielrbradley left a comment

Choose a reason for hiding this comment

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

All looks good. Perhaps let's not close the tracking issue with this PR and leave it open until we've done the follow-up PR to add the aliases?

provider/pkg/resources/resources.go Show resolved Hide resolved
@thomas11 thomas11 enabled auto-merge (squash) January 23, 2025 16:21
@thomas11 thomas11 merged commit f73a1a3 into master Jan 23, 2025
23 checks passed
@thomas11 thomas11 deleted the tkappler/v3-resolve-path-conflicts branch January 23, 2025 16:32
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v2.84.0.

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.

Disambiguate resources with different paths in any API version
3 participants