https://pulumi.com logo
Join Slack
Powered by
I am trying to understand this change: <https://g...
# typescript
w
I am trying to understand this change: https://github.com/pulumi/pulumi-aws/commit/c7326150f724a852bdb96b5fe13deb2a0c8a1104#diff-1d75aaef6e18290ff391879cc61091d6R10 which changed the return type of 
getSecret
from 
Promise<GetSecretResult>
to 
Promise<GetSecretResult> & GetSecretResult
. Turning this into an intersection type broke some of our code, and I'm not sure why this should be an intersection at all. Could someone enlighten me? Thanks!
l
Hi dustin!
I made this change. The reason for it is that we're now exposing all data sources as both async/sync.
'async' for back compat (i.e. for people that explicitly used 'then' on them)
and 'sync' because it's much much easier to just call things without having to use 'await' or be in an 'async context.
and I'm not sure why this should be an intersection at all
it's an intersection because it now is "both" and you can treat it either way you want. you can still await/.then it. or you can just directly access the properties you want off of it.
w
cc @dazzling-memory-8548
l
Can you let me know what issue you ran into in terms of typings?
for example a 
Promise<T> & T
should still be passible to something that expects a 
Promise<T>
and it can now be passed to something that expects a T
w
we are using sinonjs to stub the function, and sinon typechecks the stub that we provide against the original
our stubbed value is something like 
Promise.resolve({id: "", stringValue, "", ...})
for all the values in the secret result interface
l
Ah.
what you can do is now: 
pulumi.liftProperties(Promise.resolve({id: "", stringValue, "", ...}))
w
nice. i'll give that a try
is there a version of that where i don't have to wrap it in a promise?
i was only doing that to match the interface
l
you could technically just return the raw object (Casted to any)
however, this will only work if all your consumers are updated to only access the synchronous side of these functions
w
i'm not a fan of casting to any if i can avoid
l
i.e. they don't call .then(...)
(or, just cast to 
<Promise<Whatever> & Whatever>
basically, lie.
w
haha, i like the liftProperties idea better
i'll have to double check but i don't think we are chaining promises in our program. we do rely on outputs a lot though
i had to access via 
pulumi.utils.liftProperties
i am getting a new error with this that i'm having trouble debugging
l
what's up?
w
l
err... that is something totally new. Not sure what this is about. @microscopic-florist-22719 do you know why the monitor would be null/undefined for them?
w
pinging this 🏓
@microscopic-florist-22719
m
Are you running as under the Pulumi CLI, or are you running in test mode?
If it’s the latter, then you are probably hitting the issue you linked.
w
running in test mode
do you know of a workaround?
m
Hmm. You could monkey-patch the implementation of 
invoke
to skip the actual call...
You mentioned a stubbing library
That would also work, I think?
w
yeah we're using sinon
m
I’m not familiar with Simon
*sinon
(sorry, phone-bound)
Will it let you stub out arbitrary functions?
w
Note that in PULUMI_TEST_MODE you quite generally cannot call 
.getFoo
methods succesfully. So you either need to monkey patch these, or use a technique more like https://www.pulumi.com/blog/unit-testing-infrastructure-in-nodejs-and-mocha/ to run tests during a real 
preview
and/or 
update
.
w
i'll have to dig into the pulumi internals a bit to understand what/how to stub
we are using that blog post as a guide
our tests were running before the upgrade to pulumi 0.17.23 and pulumi/aws 0.18.23
if i bump @pulumi/aws to 0.18.23 i get that error
otherwise i don't
m
I wonder if a method name or signature changed...
Do you have sinon configured to stub out 
getSecretVersion
?
Also, what version of 
@pulumi/aws
is in your package-lock.json or yarn.lock?
w
we do not stub out getSecretVersion all the time
do you want the locked version when the error occurs? i can get it back to a reproducible state and get that info
ok, i did
Copy code
npm i --save @pulumi/aws@0.18.23
ran tests and got the error
Copy code
> npm list @pulumi/aws
aws-typescript@ /Users/dustinfarris/Work/DLP/pulumi-ucboitlake
├── @pulumi/aws@0.18.23
├─┬ @pulumi/awsx@0.18.7
│ └── @pulumi/aws@0.18.23  deduped
└─┬ pulumi-lib-aws@0.1.0 (<git+ssh://git@stash.int.colorado.edu:7999/dlp/pulumi-lib-aws.git#c95252757ebb563bf18f78f016aabb22869cdac4>)
  └── @pulumi/aws@0.18.20
m
Can you stub out 
getSecretVersion
all the time when running tests?
I'm also wondering if the two versions of 
@pulumi/aws
are going to cause problems here
(perhaps only one version of 
getSecretVersion
is stubbed)
w
will try stubbing it every time and see where that gets me
I think I'm starting to understand — we've been getting this error all along, but until now, it was buried in a promise that never resolved; so, the error was getting swallowed by 
UnhandledPromiseRejectionWarning
. As a warning, this output was a nuisance, but did not prevent us from running our tests. cc @gentle-diamond-70147 https://github.com/pulumi/pulumi/issues/2838 With the new synchronous features being introduced into Pulumi, this error is no longer trapped by a Promise, it's just an error outright, which is why our tests don't run anymore. https://github.com/pulumi/pulumi/issues/2921 is really a dupe of 2838 minus the unhandled promise rejection warning.
Untitled
w
Good point on 2921 being a dupe of 2838 - I have closed that one out. Note that we wrote another blog post on an alternate unit testing approach that doesn't have this fundamental limitation at https://www.pulumi.com/blog/unit-testing-infrastructure-in-nodejs-and-mocha/. This works quite differently than the 
PULUMI_TEST_MODE
approach you are using. Would love your thoughts on whether that helps for your testing scenarios.
2 Views
Open in Slack
PreviousNext
Powered by