Add pulumi.Asset support to lambda function #182
Conversation
| @@ -828,6 +828,11 @@ func (ctx *context) genProperties(parentName string, typeSchema *jsschema.Schema | |||
| Description: value.Description, | |||
| TypeSpec: *typeSpec, | |||
| } | |||
| if parentName == "FunctionCode" && name == "ZipFile" { | |||
There was a problem hiding this comment.
I think we rename a property in Google native to something like contents. I'm not sure we want to do this here... but worth discussing?
lblackstone
force-pushed
the
lblackstone/lambda-assets
branch
3 times, most recently
from
0d626a5
to
7c733a5
Compare
lblackstone
force-pushed
the
lblackstone/lambda-assets
branch
from
683944a
to
e4381ea
Compare
| const updated = { "response": response + "World!" }; | ||
| callback(null, updated); | ||
| };`, | ||
| zipFile: new pulumi.asset.FileAsset("world_function.js"), |
There was a problem hiding this comment.
That's why I'm not a fan of the zipFile property... There's no zip in here, right? Should we rename to code, as in the classic provider? The motivation is that we are changing the behavior of a property, so we may rename it as well.
There was a problem hiding this comment.
I'm conflicted on this. I agree that the name is confusing, but I'm also hesitant to change the name at the native provider layer. I would be more open to adding an alias, perhaps. My main concern is that we should make it easy for integrators to build on top of the native layer, so I think we should avoid changing the API in incompatible ways.
There was a problem hiding this comment.
Okay, that's reasonable. Let's proceed with the current name for now then.
There was a problem hiding this comment.
Should we consider updating the description field in the schema for this attribute to make the confusing semantics clearer?
|
Thinking of this again, I think we shouldn't ship this until we are sure we can support archives (which it sounds like we may not be able to). |
Converted the PR to draft so we don't merge accidentally. |
|
@lblackstone I'm wondering if there is any update on the status of this PR. It looks like it's halted, while so much progress was made. Having support for the pulumi asset functionality would be really nice. |
It's been a long time since I looked at this, but my recollection is that the upstream Cloud Control API didn't support this, which is why it stalled out. I'm not sure if that has changed in the meantime. Maybe @danielrbradley knows? |
|
I wasn't involved in this the first time around, but my inital though looking through is that we're going to exceed the field size limit quite quickly using this approach, but maybe this would still have value for small use cases. An alternative approach might be to wrap up the upload of the assets to S3 within a component though I'm not exactly sure where this would live - perhaps a separate component provider? |
|
A second thought on this PR ... we should move the asset version into an alternative field to ensure the resource aligns to the underlying spec, but then we have an additional Asset field which can be used to populate the ZipFile string from an asset for convinience while maintaining compatability. |
|
@mjeffryes claims this one is stale. Should we close? Any plans to carry forward? |
|
@t0yv0 I think this is useful to keep around as it's still an accurate summary of the changes needing to be made to tie this through. I do think we're still wanting to support Assets in some capacity, though I'd suggest that this might be either:
|
Fix #124