-
Notifications
You must be signed in to change notification settings - Fork 159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DO NOT MERGE: Fiddle with serialized ProviderInfo #2902
DO NOT MERGE: Fiddle with serialized ProviderInfo #2902
Conversation
This is good. The "ultimate" would be to do this while also avoiding JSON. That is we code-gen "MarshallableProviderInfo" directly into Go structures and evaluate them instead of the JSON. If JSON is still any meaningful overhead. |
You're right pointing out that info.P is missing and needs to be populated. There's one more problem. info.P maps are mutable and there's at least one form of aliasing using that mutation. This will not survive Marshal/Unmarshal pass since it's not serializable. We need to reify the mutations into a data structure to replay them. |
From Slack:
|
Nice; I must have missed or glossed your comment in slack before and wasted some effort trying to re-derive it. Glad we are thinking in the same direction anyway. I also looked a bit at binary encoding and code generation for the format; jsoniter is fast enough for now, but it would be good to get the plugin init time down under 100ms. I'll keep exploring. |
Yes code generation might be perfect. Especially if it can inject some form of laziness so sub-resource information is loaded/computed on demand. |
@t0yv0 it looks like we are marshalling the Schema, ResourcesMap and DataSourcesMap from info.P (http://github.com/pulumi/pulumi-terraform-bridge/blob/d74f91f8cd3946ac260fb1767202d176ff3ea412/pkg/tfbridge/info.go#L851-L851 |
We're marshalling them but that's not going to turn aroud unmarshalling.
'mostly' is doing a lot in this sentence. It will not be usable at runtime, it can only be useful for static analysis. |
That said to your quetion, yes those are the ones. They are instances of shim.SchemaMap. https://github.com/pulumi/pulumi-terraform-bridge/blob/master/pkg/tfshim/shim.go#L147 Currently this interface has a nefarious Set(key string, value Schema) method. Otherwise it would be immutable. With the kind of surgery you're trying to do here, it's very easy to lose track of these Set invocations so they would only happen at tfgen time an be lost at runtime. My initial thought was that probably we could presrve them by capturing them into a datastructure at tfgen time and replaying these edits at runtime. Perhaps you're suggesting that instead we can get correctly edited maps from an unmarshallable provider. Since the marshallable provider is unusable, we'd have to hybridize it with the actual live provider by doing some map swapping. This makes me a little nervous since object graph might matter, these are not pure value semantics. |
My plan is to try this and see what looks broken. |
What will be broken is aliases. This: On a live provider, ResourceMap contains resources that have schema AND live behavior so they can execute Create calls. If we edit ResourceMap for aliasing we're copying these live resource objects references from one key to another. If we're marshaling ResourceMap the liveness of the resources gets lobotomized and they're replaced with schema-only dummies. If we un-marshal it back we get the dummies in the map that can't serve Create calls. The key space of the map correctly copied the dummies but the resources in the original live map are not copied. Now we need to reconcile the two so we get correct key space - to correct resource mappings but every dummy resource is replaced with a live resource. Perhaps doable.. |
68683f7
to
394d269
Compare
updated this to use the experiment in pulumi/pulumi-terraform-bridge#1475 (it's not going to build though because I left in some replaces from my local testing) |
Ian pointed out that this is probably a fruitless direction to pursue because there are too many non-serializable hooks in the ResourceInfo that are needed at runtime. We will have to take a more piecemeal approach. |
That's a good observation. Things like Transformer, yes, will not survive a roundtrip. |
Played with serializing the whole ProviderInfo to see how much we could gain from pulumi/pulumi-terraform-bridge#1454; on my machine this will init in about the same time as the v5 plugin (though not any faster).
Obviously, we have to init the actual provider too; so this is a better than realistic case.
I still need to test how close
MarshalledProviderInfo
gets us to theProviderInfo
generated in resources.go, but I could see building on this as a generic way to separate generation code from runtime code.