https://pulumi.com logo
b

bored-oyster-3147

03/11/2021, 9:35 PM
@lemon-agent-27707 @tall-librarian-49374 looking for thoughts on something. Currently I am wiring up the Pulumi Automation preview to our internal CD app and we're looking to preserve that pulumi log output for historical purposes. So we'll probably hook into that
OnOutput
delegate to capture those logs. But currently if you look here you'll see that the
OnOutput
delegate is only invoked when the StdOut stream is written to and not when StdErr is written to. I believe I just mimicked the behavior from the other SDKs when I did that.. but that means that if their is an error it will only be reflected in the resulting exception and not in our historical logs that we built from the
OnOutput
delegate. Do you think that maybe we should have a second delegate for
OnError
, or maybe
OnOutput
should be invoked for both?
I could pull it off of this but that won't preserve the ordering of the logs as they occurred
r

red-match-15116

03/12/2021, 1:25 AM
Hey! I definitely think this is worth doing, opened https://github.com/pulumi/pulumi/issues/6511 to track.
I think we’re going to take the break here since this is the whole point of being in preview so we can streamline for GA. @bored-oyster-3147 would you be willing to make the change for C#?
b

bored-oyster-3147

03/12/2021, 2:02 AM
Yea I can do that tomorrow!
🙌 1
🙏 1
r

red-match-15116

03/12/2021, 2:58 AM
Little change of plans, I think we’d rather avoid the break. I updated the issue description, adding a delegate for
OnStderr
for C# should be all that needs to happen.
b

bored-oyster-3147

03/12/2021, 4:56 AM
Would we want to name it
OnError
instead then? To keep with the
OnOutput
naming convention? Seems weird to go half way with the names, if you’re gonna break it later to re-name one why not do both later - right?
t

tall-librarian-49374

03/12/2021, 8:58 AM
p

prehistoric-coat-10166

03/12/2021, 9:29 AM
Hey everyone! Author of that PR (6513) here, I only saw this thread now, so hopefully I did not waste anyone's time
b

bored-oyster-3147

03/12/2021, 1:45 PM
I think to get it to write to StdErr you’d want to cause one of the pulumi exceptions, such as stack already exists exception
Which wouldn’t occur during an update action lol sooo probably like a resource missing required parameters then?
p

prehistoric-coat-10166

03/12/2021, 1:50 PM
I'll see if that would trigger it, I tried for example using
Config
and requiring a value that does not exist, but even that error would end up being written to the Stdout
b

bored-oyster-3147

03/12/2021, 1:52 PM
Oh, huh. Well then I don't think required resource parameters will do it either. One of the pulumi guys might have a better example
p

prehistoric-coat-10166

03/12/2021, 1:56 PM
From what I see, currently all of the normal Pulumi output is being written to Standard output, including errors
This also includes writing via
System.Console.Error
in C# inside a Pulumi program, which normally would write to standard error
l

lemon-agent-27707

03/12/2021, 3:47 PM
I would say generally that if Pulumi has incorrect or buggy stderr hygiene in some places that we should consider it a bug. I would not want to design this API around that bug. stdout and stderr refers the the CLI process which will surface output form other processes (language runtime, provider plugins).
p

prehistoric-coat-10166

03/12/2021, 3:50 PM
I think this should then be ok as it is (with some minor naming changes) and then we can create another issue regarding the standard error hygiene for the CLI, which should then propagate through the automation API. How does that sound?
👍 2
Created https://github.com/pulumi/pulumi/issues/6515 regarding the stderr hygiene, lemme know if you need more info
r

red-match-15116

03/12/2021, 4:53 PM
I added a little more context in the issue around why I would like to avoid calling this handler
OnError
and be clear about the fact that it is actually
OnStandardError
. https://github.com/pulumi/pulumi/issues/6511
p

prehistoric-coat-10166

03/12/2021, 4:56 PM
Is it okay to name
OnOutput
as
OnStandardOutput
as well?
This would result in a breaking change, but it would then be done and consistent
r

red-match-15116

03/12/2021, 5:09 PM
I guess since the C# SDK is relatively new and unlikely to have heavy usage (apart from you fine folk) just yet I would be okay with taking the breaking change to rename
OnOutput
to
OnStandardOutput
. I would request that you also update the usage in the examples if going with
OnStandardOutput
: https://github.com/pulumi/automation-api-examples#net-examples 🙏
🙌 1
p

prehistoric-coat-10166

03/12/2021, 5:23 PM
will do!
🙌 1
🙏 1