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

Respect schema versions (toggled via flag) #8881

Merged
merged 10 commits into from
Feb 3, 2022
Merged

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Jan 31, 2022

Description

Fixes #8521

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

@iwahbe iwahbe self-assigned this Jan 31, 2022
@github-actions
Copy link

Diff for pulumi-random with merge commit 5eaa476

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 5eaa476

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 5eaa476

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 5eaa476

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 678fc2c

@github-actions
Copy link

Diff for pulumi-random with merge commit 678fc2c

@github-actions
Copy link

Diff for pulumi-azure with merge commit 5eaa476

@iwahbe iwahbe mentioned this pull request Jan 31, 2022
3 tasks
@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 678fc2c

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 678fc2c

@github-actions
Copy link

Diff for pulumi-azure with merge commit 678fc2c

@github-actions
Copy link

Diff for pulumi-aws with merge commit 5eaa476

@github-actions
Copy link

Diff for pulumi-aws with merge commit 678fc2c

@codecov
Copy link

codecov bot commented Jan 31, 2022

Codecov Report

Merging #8881 (f8f5cdf) into master (772953d) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8881      +/-   ##
==========================================
+ Coverage   59.35%   59.41%   +0.05%     
==========================================
  Files         642      642              
  Lines       99555    99651      +96     
  Branches     1389     1389              
==========================================
+ Hits        59093    59203     +110     
+ Misses      37090    37074      -16     
- Partials     3372     3374       +2     
Impacted Files Coverage Δ
pkg/codegen/dotnet/importer.go 44.00% <ø> (ø)
pkg/codegen/go/importer.go 18.75% <ø> (ø)
pkg/codegen/nodejs/importer.go 40.00% <ø> (ø)
pkg/codegen/python/importer.go 30.00% <ø> (ø)
pkg/codegen/dotnet/gen.go 82.52% <100.00%> (+0.06%) ⬆️
pkg/codegen/go/gen.go 89.72% <100.00%> (+0.01%) ⬆️
pkg/codegen/nodejs/gen.go 84.10% <100.00%> (+0.10%) ⬆️
pkg/codegen/python/gen.go 84.37% <100.00%> (+0.07%) ⬆️
pkg/codegen/python/utilities.go 92.17% <100.00%> (+8.84%) ⬆️
pkg/codegen/hcl2/model/type_object.go 89.72% <0.00%> (+0.54%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 772953d...f8f5cdf. Read the comment docs.

}

if pkg.Version != nil && codegen.RespectVersion() {
files.add("version.txt", []byte(pkg.Version.String()))
Copy link
Member

Choose a reason for hiding this comment

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

What is version.txt supposed to be? If we're stamping a version to the codegen'd project should it be stamped into the project.csproj as ?

Copy link
Member Author

Choose a reason for hiding this comment

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

version.txt is the file we use to check the version of the package (at runtime). It makes some amount of sense to stamp the version into *.csproj.

pkg/codegen/utilities.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 5eaa476

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 678fc2c

@t0yv0
Copy link
Member

t0yv0 commented Jan 31, 2022

Something going on with tests:

=== Failed
=== FAIL: codegen/dotnet TestGeneratePackage/simple-resource-schema (0.00s)
    sdk_driver.go:367: Simple schema with local resource properties
    sdk_driver.go:386: 
        	Error Trace:	sdk_driver.go:386
        	Error:      	Received unexpected error:
        	            	Failed to load file version.txt referenced in codegen-manifest.json: open ../internal/test/testdata/simple-resource-schema/dotnet/version.txt: no such file or directory
        	Test:       	TestGeneratePackage/simple-resource-schema
    --- FAIL: TestGeneratePackage/simple-resource-schema (0.00s)

=== FAIL: codegen/dotnet TestGeneratePackage (181.51s)

Perhaps the version.txt was not checked in?

Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

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

Left a few comments but generally this looks quite useful - LGTM!

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-random with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-azure with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-aws with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 77fd427

@github-actions
Copy link

Diff for pulumi-azuread with merge commit e9a00d7

@github-actions
Copy link

Diff for pulumi-random with merge commit e9a00d7

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit e9a00d7

@github-actions
Copy link

Diff for pulumi-gcp with merge commit e9a00d7

@github-actions
Copy link

Diff for pulumi-azure with merge commit e9a00d7

@github-actions
Copy link

Diff for pulumi-aws with merge commit e9a00d7

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit e9a00d7

Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

pkg/codegen/nodejs/gen.go Outdated Show resolved Hide resolved
pkg/codegen/dotnet/gen.go Show resolved Hide resolved
Copy link
Contributor

@stack72 stack72 left a comment

Choose a reason for hiding this comment

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

This is a great way to roll out new functionality - opt-in to start with. As this is off by default, I can't see any issues here!

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azuread with merge commit 67d6910

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-random with merge commit 67d6910

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-kubernetes with merge commit 67d6910

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-gcp with merge commit 67d6910

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azure with merge commit 67d6910

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-aws with merge commit 67d6910

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azure-native with merge commit 67d6910

@iwahbe iwahbe merged commit f2d8858 into master Feb 3, 2022
@pulumi-bot pulumi-bot deleted the iwahbe/respect-versions-v3 branch February 3, 2022 16:07
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.

[Codegen] generated packages do not respect the specified version
6 participants