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

Pass opts to wrapped Pulumi resources #621

Closed
remembell-root opened this issue Jun 30, 2024 · 8 comments
Closed

Pass opts to wrapped Pulumi resources #621

remembell-root opened this issue Jun 30, 2024 · 8 comments
Assignees

Comments

@remembell-root
Copy link

In many of the custom resources you use to wrap Pulumi's resources (SnsTopic, S3, etc) you don't pass the opts, and I see it as an error that needs fixing.

For example, in SnsTopic the code is:

function createTopic() {
  return new aws.sns.Topic(`${name}Topic`, transform(args.transform?.topic, { fifoTopic: fifo }), { parent });
}

so, aws.sns.Topic only gets { parent } as opts.

It can be fixed by doing something similar to:

function createTopic() {
  return new aws.sns.Topic(`${name}Topic`, transform(args.transform?.topic, { fifoTopic: fifo }), { ...opts, parent });
}
@joshwils82
Copy link

This would also allow a workaround for #617

@jayair
Copy link
Contributor

jayair commented Jul 3, 2024

Hmm interesting. We'll need to consider the case when there are multiple resources inside a component.

@dror-weiss
Copy link

@jayair , since there is a transform options for Pulumi args you could (breaking) change to transformArgs and transformOpts. This will solve the case for multiple resources inside a component and be much clearer (in my opinion).

@fwang
Copy link
Contributor

fwang commented Jul 28, 2024

@remembell-root Which opts field are you looking to pass to child resources?

so, aws.sns.Topic only gets { parent } as opts.

Pulumi passes a subset of opts to child resources by default, ie. retainOnDelete

@fwang
Copy link
Contributor

fwang commented Jul 28, 2024

In the latest version of SST, u can transform opts like this

new sst.aws.Bucket("MyBucket", {
  transform: {
    bucket: (args, opts) => {
      opts.retainOnDelete = true;
    },
  },
});

@fwang fwang closed this as completed Jul 28, 2024
@dror-weiss
Copy link

@fwang, thanks for implementing this.

I'm sorry, I don't understand why use function transform and not pass the entire opts from the constructor (as suggested).

@jayair
Copy link
Contributor

jayair commented Aug 2, 2024

Cos the bucket component has multiple resources in it

@dror-weiss
Copy link

@jayair , could you please make notes for next major release to discuss the api change I suggested? It will make my code much cleaner and simpler to understand.

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

No branches or pull requests

5 participants