https://pulumi.com logo
Title
b

bored-oyster-3147

12/15/2020, 6:48 PM
Gonna make a new thread @lemon-agent-27707 - thoughts on this: The StackSettings object has a property
IDictionary<string, StackSettingsConfigValue> Config { get; }
. The
StackSettingsConfigValue
looks like:
{
    string? ValueString,
    IDictionary<string, object>? ValueObject,
    bool IsSecure,
    bool IsObject
}
This allows us to deserialize a structured object provided to stack settings config in a shape that isn't known at compile time. There is a catch though. Since both
System.Text.Json
and
YamlDotNet
deserialize to a dictionary differently, the compile-time type of
ValueObject
must remain
Dictionary<string, object>
in order to support both formats, since we don't want JSON or YAML to be exposed on
StackSettingsConfigValue
. But the runtime-type will be different depending on what format we are deserializing. Coming from JSON with
System.Text.Json
, the type of
TValue
at runtime will always be
JsonElement
. For good measure I tried
Newtonsoft.Json
, and this library tries to determine the type if it is a primitive, but only at the first level of depth. So if the property is a .NET primitive it will be that type - I tried and got
long
or
double
for numbers depending on if there is a decimal, as well as
bool
&
string
. But if the property is itself an object, than rather than getting another nested
Dictionary
you instead receive Newtonsoft's type
JObject
. Coming from YAML with
YamlDotNet
, the type of
TValue
at runtime is always a
string
, but it will break apart nested objects. So you receive either
string
or
Dictionary<string, string>
depending on if the property itself is an object. So questions, are we OK with this implementation? Are we OK with the runtime type being different depending on which format we're deserializing from? Do we prefer Newtonsoft's method over System.Text.Json? Honestly I prefer System.Text.Json's just because it is consistent across all depths, but it does mean that the json-specific Type is exposed right away (at runtime).
l

lemon-agent-27707

12/15/2020, 9:38 PM
@tall-librarian-49374 any thoughts here? It is not desirable for users to manipulate or read config via this api. The goal of this api is really to be able to whole sale save or write a pulumi.stack.yaml/json. Any manipulation of config should happen through the config APIs, as those will perform all necessary encryption/decryption.
b

bored-oyster-3147

12/15/2020, 9:42 PM
well in this scenario I think if they
GetStackSettings
and then
SaveStackSettings
with the same object after making any changes (even if those changes weren't to config) they will possibly have issues. If it was JSON, the save will try to serialize
JsonElement
which I think will be fine since
System.Text.Json
can do that out of the box. But if it is YAML than they may lose some context since everything is deserialized as a
string
and will then be serialized as such
t

tall-librarian-49374

12/15/2020, 10:23 PM
It sounds a bit odd to get different behavior depending on the source type JSON/YAML. I guess I’d expect the in-memory representation to operate the same set of primitives as our protobuf serialization in https://github.com/pulumi/pulumi/tree/master/sdk/dotnet/Pulumi/Serialization (which includes JsonElement and immutable dictionaries)
b

bored-oyster-3147

12/15/2020, 10:33 PM
so can you explain what you think the resulting dictionary from each format should be? That means I need to do some additional work after deserializing from JSON or YAML to get to the target dictionary.
t

tall-librarian-49374

12/15/2020, 10:36 PM
I wouldn’t spend too much time on this right now if that’s an “internal” API. Unless this is required for correctness on load/save.
b

bored-oyster-3147

12/15/2020, 10:39 PM
that's the issue - it is required for Get/Save of ProjectSettings and StackSettings
l

lemon-agent-27707

12/15/2020, 10:59 PM
I agree that this API isn't that important. I think it would be fine for V1 to say that if you want to work with stack settings or project settings, you have to do it via interacting with the files directly in the
WorkDir
.
b

bored-oyster-3147

12/15/2020, 11:04 PM
So if it is anything other than a plain string or secure string, we could just not deserialize it?
l

lemon-agent-27707

12/16/2020, 12:11 AM
I guess I’m suggesting not to implement the project settings or stack settings APIs for the time being.
b

bored-oyster-3147

12/16/2020, 12:33 AM
Hmmmm I think we can do that. It doesn’t look like anything else depends on them. It’s your call
l

lemon-agent-27707

12/16/2020, 4:26 PM
Let's go ahead and defer on implementation here in favor of making progress on end to end scenarios.