https://pulumi.com logo
Title
t

tall-needle-56640

04/01/2021, 4:22 PM
Can this code be simplified?
foreach (var element in Environment.GetEnvironmentVariables())
{
    if (element is KeyValuePair<string, object> pair
        && pair.Value is string valueStr)
        env[pair.Key] = valueStr;
}
Environment.GetEnvironmentVariables
returns an
IDictionary
. Ideally, it would be typed, but generics probably hadn't been introduced yet. I think it's obvious that it should actually return
IDictionary<string, string?>
, but in case of any doubt
Environment.GetEnvironmentVariable(string)
only returns
string?
, not
object
. So what value does
element is KeyValuePair<string, object>
give us? Therefore, couldn't the code be simplified to
foreach (var pair in Environment.GetEnvironmentVariables())
{
    if (pair.Value is string valueStr)
        env[pair.Key] = valueStr;
}
@bored-oyster-3147
b

bored-oyster-3147

04/01/2021, 4:25 PM
Maybe? But I don't know if we know the type of
pair.Key
is
string
at compile-time so
env[pair.key] = ...;
might give you an error?
Here's your answer I think:
We need to get the KVP first.
Did you find that we were losing environment variables doing it this way?
t

tall-needle-56640

04/01/2021, 4:44 PM
Ah, that makes sense. There's no problems, I was just confused why the second type was
object
and not
string
.
b

bored-oyster-3147

04/01/2021, 5:17 PM
Have you personally tried passing environment variables from your system into pulumi? I was just messing with this code in a console app and I found some issues (thanks for bringing it up, lol). First of all,
element
is of type
System.Collection.DictionaryEntry?
. And none of them satisfied
KeyValuePair<string, object>
,
KeyValuePair<string, string>
, or
KeyValuePair<string, string?>
... which is concerning I actually had to do this to get working:
var env = Environment.GetEnvironmentVariables();
            foreach (DictionaryEntry? element in env)
            {
                if (element is null || !element.HasValue)
                    continue;

                if (element.Value.Key is string keyStr
                    && element.Value.Value != null
                    && element.Value.Value is string valueStr)
                    Console.WriteLine($"[{keyStr}] = {valueStr}");
            }
@enough-garden-22763 this may be an oversight on my part we might want a test that system environment vars are making it into pulumi execution, I think my
is KeyValuePair<string, object>
check might never be satisfied currently
p

prehistoric-coat-10166

04/01/2021, 5:25 PM
But yeah
foreach (DictionaryEntry entry in Environment.GetEnvironmentVariables())
should do the trick and then checking the key & value
b

bored-oyster-3147

04/01/2021, 5:26 PM
that's a pretty big faux pas on my part yikes
@prehistoric-coat-10166 unfortunately with the nullable reference types enabled that function is returning nullable dictionary entries for some reason so that added null check complexity in my example above is necessary
t

tall-needle-56640

04/01/2021, 5:29 PM
This is redundant:
element.Value.Value != null
                    && element.Value.Value is string valueStr
If it's null, it won't say it's a string
b

bored-oyster-3147

04/01/2021, 5:31 PM
you are correct, that is because I was doing this first and was getting an error and switched to what I posted
p

prehistoric-coat-10166

04/01/2021, 5:33 PM
if (element.Value.Key is string keyString
                    && element.Value.Value is string valueString)
                {
                    env[keyString] = valueString;
                }
Should work (with the element null check)
e

enough-garden-22763

04/01/2021, 5:38 PM
Hey team just seeing this discussion here 👋
System environment vars are making it into pulumi execution indeed, we don’t have a test checked in but some test I’ve done indicate its’ happening.
b

bored-oyster-3147

04/01/2021, 5:45 PM
Cool so we should be good with your PR, then. Awesome.
p

prehistoric-coat-10166

04/01/2021, 5:46 PM
Yeah, deleted code is best code!
🙌 1
Do we want to still allow the nullable environment variables on our end? With for example default value of empty to consistently "unset" environment variables until CliWrap allows nulls?
Although I guess it might just as easy on the users' side to use
ValueWhichMayBeNull ?? ""
Heh, actually the problem seems to go deeper to
Process
https://github.com/dotnet/runtime/issues/34446