https://pulumi.com logo
Powered by
w

worried-engineer-33884

07/23/2019, 5:04 PM
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

lemon-spoon-91807

07/23/2019, 7:01 PM
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

worried-engineer-33884

07/23/2019, 7:11 PM
cc @dazzling-memory-8548
l

lemon-spoon-91807

07/23/2019, 7:12 PM
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

worried-engineer-33884

07/23/2019, 7:13 PM
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

lemon-spoon-91807

07/23/2019, 7:13 PM
Ah.
what you can do is now: 
pulumi.liftProperties(Promise.resolve({id: "", stringValue, "", ...}))
w

worried-engineer-33884

07/23/2019, 7:16 PM
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

lemon-spoon-91807

07/23/2019, 7:17 PM
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

worried-engineer-33884

07/23/2019, 7:17 PM
i'm not a fan of casting to any if i can avoid
l

lemon-spoon-91807

07/23/2019, 7:17 PM
i.e. they don't call .then(...)
(or, just cast to 
<Promise<Whatever> & Whatever>
basically, lie.
w

worried-engineer-33884

07/23/2019, 7:18 PM
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

lemon-spoon-91807

07/23/2019, 8:15 PM
what's up?
w

worried-engineer-33884

07/23/2019, 8:15 PM
l

lemon-spoon-91807

07/23/2019, 8:20 PM
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

worried-engineer-33884

07/24/2019, 11:40 AM
pinging this 🏓
@microscopic-florist-22719
m

microscopic-florist-22719

07/24/2019, 8:14 PM
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

worried-engineer-33884

07/24/2019, 8:18 PM
running in test mode
do you know of a workaround?
m

microscopic-florist-22719

07/24/2019, 8:28 PM
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

worried-engineer-33884

07/24/2019, 8:29 PM
yeah we're using sinon
m

microscopic-florist-22719

07/24/2019, 8:29 PM
I’m not familiar with Simon
*sinon
(sorry, phone-bound)
Will it let you stub out arbitrary functions?
w

white-balloon-205

07/24/2019, 8:36 PM
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

worried-engineer-33884

07/24/2019, 8:36 PM
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

microscopic-florist-22719

07/24/2019, 8:42 PM
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

worried-engineer-33884

07/24/2019, 8:51 PM
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

microscopic-florist-22719

07/24/2019, 9:50 PM
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

worried-engineer-33884

07/24/2019, 9:52 PM
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

white-balloon-205

07/25/2019, 4:40 PM
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
Powered by