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.

Sign up for GitHub

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

Jump to bottom

Fix l2-resource-config / plugin download URL code generation #1566

Closed
lunaris opened this issue Jan 6, 2025 · 1 comment · Fixed by #1586 or #1595
Closed

Fix l2-resource-config / plugin download URL code generation #1566

lunaris opened this issue Jan 6, 2025 · 1 comment · Fixed by #1586 or #1595
Assignees
@Zaid-Ajaj
Labels
kind/bug Some behavior is incorrect or out of spec language-host Runtime that executes user programs resolution/fixed This issue was fixed

Comments

@lunaris
Copy link
Contributor

lunaris commented Jan 6, 2025

The l2-resource-config conformance test currently fails due to some values not lining up. It looks like we are not respecting plugin download URLs (at least -- there may be other things we are missing).

=== NAME  TestLanguage/l2-resource-config
    language_test.go:146: l2_resource_config.go:56: 
                Error Trace:    /home/will/git/pulumi/pulumi-java-main/pulumi/cmd/pulumi-test-language/tests/l2_resource_config.go:56
                                                        /home/will/git/pulumi/pulumi-java-main/pulumi/cmd/pulumi-test-language/interface.go:1098
                                                        /home/will/git/pulumi/pulumi-java-main/pulumi/cmd/pulumi-test-language/tests/types.go:182
                                                        /nix/store/chzgk756zb2cqlzbjr86m0lfxi63cdfy-go-1.22.7/share/go/src/runtime/asm_amd64.s:1695
                Error:          Not equal: 
                                expected: resource.PropertyMap{"__internal":resource.PropertyValue{V:resource.PropertyMap{"pluginDownloadURL":resource.PropertyValue{V:"http://example.com"}}}, "name":resource.PropertyValue{V:"my config"}, "pluginDownloadURL":resource.PropertyValue{V:"not the same as the pulumi resource option"}, "version":resource.PropertyValue{V:"9.0.0"}}
                                actual  : resource.PropertyMap{"__internal":resource.PropertyValue{V:resource.PropertyMap{}}, "name":resource.PropertyValue{V:"my config"}, "pluginDownloadURL":resource.PropertyValue{V:"not the same as the pulumi resource option"}, "version":resource.PropertyValue{V:"9.0.0"}}
                            
                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -2,6 +2,3 @@
                                  (resource.PropertyKey) (len=10) "__internal": (resource.PropertyValue) {
                                -  V: (resource.PropertyMap) (len=1) {
                                -   (resource.PropertyKey) (len=17) "pluginDownloadURL": (resource.PropertyValue) {
                                -    V: (string) (len=18) "http://example.com"
                                -   }
                                +  V: (resource.PropertyMap) {
                                   }
        
    language_test.go:148: stdout: Created stack 'test'
        
         +  pulumi:pulumi:Stack l2-resource-config-test creating (0s) 
         +  pulumi:providers:config prov creating (0s) 
         +  pulumi:providers:config prov created (0.00s) 
         +  config:index:Resource res creating (0s) 
         +  config:index:Resource res created (0.00s) 
         +  pulumi:pulumi:Stack l2-resource-config-test created (0.44s) 
        Outputs:
            pluginDownloadURL: "not the same as the pulumi resource option"
        
        Resources:
            + 3 created
        
        Duration: 3s
        
    language_test.go:149: stderr: 
    language_test.go:150: 
                Error Trace:    /home/will/git/pulumi/pulumi-java-main/pkg/cmd/pulumi-language-java/language_test.go:150
                Error:          Should be true
                Test:           TestLanguage/l2-resource-config
--- FAIL: TestLanguage (10.95s)
    --- FAIL: TestLanguage/l2-resource-config (12.69s)
FAIL
FAIL    github.com/pulumi/pulumi-java/pkg/cmd/pulumi-language-java      23.662s
FAIL
@lunaris lunaris added kind/bug Some behavior is incorrect or out of spec language-host Runtime that executes user programs labels Jan 6, 2025
@Zaid-Ajaj Zaid-Ajaj self-assigned this Jan 9, 2025
This was referenced Jan 10, 2025
Merged
Merged
Zaid-Ajaj added a commit that referenced this issue Jan 14, 2025
@Zaid-Ajaj
[sdk] Implement InvokeOptionsBuilder and InvokeOutputOptionsBuilder (#…
5f5020d 
…1585)

### Description

The PR implements the following builder classes
 - `InvokeOptionsBuilder` for `InvokeOptions`
 - `InvokeOutputOptionsBuilder` for `InvokeOutputOptions`

It adds `pluginDownloadURL` for both of them.

This is a required in order to invoke options for program-gen for #1566

### API

Example usage for `InvokeOptionsBuilder`
```java
InvokeOptions.builder() // returns InvokeOptionsBuilder
  .parent(...)
  .provider(...)
  .pluginDownloadURL(...)
  .version(...)
  .build() // returns InvokeOptions
```

Example usage for `InvokeOutputOptionsBuilder`

```java
(new InvokeOutputOptionsBuilder())
  .parent(...)
  .provider(...)
  .pluginDownloadURL(...)
  .version(...)
  .dependsOn(...) 
  .build() // returns InvokeOutputOptions
```

The reason `InvokeOutputOptionsBuilder` cannot be initialized via
`InvokeOutputOptions.builder()` is because the `builder()` method would
conflict with the `InvokeOptions.builder()` method.

For the time being, we are omitting `.builder()` until we no longer have
`InvokeOutputOptions` _extending_ `InvokeOptions.`
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 14, 2025
@Zaid-Ajaj Zaid-Ajaj removed the resolution/fixed This issue was fixed label Jan 15, 2025
@Zaid-Ajaj
Copy link
Contributor

PR #1586 didn't actually unskip the test l2-resource-config so I am reopening the issue. It is more the fact that we are not emitting the plugin download URL from the schema into the default resource options when instantiating resources

@Zaid-Ajaj Zaid-Ajaj reopened this Jan 15, 2025
@Zaid-Ajaj Zaid-Ajaj mentioned this issue Jan 15, 2025
Merged
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Zaid-Ajaj Zaid-Ajaj

Labels
kind/bug Some behavior is incorrect or out of spec language-host Runtime that executes user programs resolution/fixed This issue was fixed
Projects
None yet
Milestone
No milestone
3 participants
@lunaris @Zaid-Ajaj @pulumi-bot