Azure / azure-functions-host

The host/runtime that powers Azure Functions
https://functions.azure.com
MIT License
1.91k stars 435 forks source link

Need assembly resolution when loading custom bindings #1497

Open MikeStall opened 7 years ago

MikeStall commented 7 years ago

For BYOB, none of the custom bindings are loading because the binding has a reference to Webjobs version X, and the script runtime refers to WebJobs version Y. We need to resolve so they all unify on Y.

Else none of the attributes and types line up, and we fail type checks like this: https://github.com/Azure/azure-webjobs-sdk-script/blob/a1360cb2a3314dea8ba806d71690d6e6c26fea72/src/WebJobs.Script/Host/ScriptHost.cs

MikeStall commented 7 years ago

This is related to https://github.com/Azure/azure-webjobs-sdk-script/issues/1397, but this bug is specifically validating the startup path in ScriptHost.

fabiocav commented 7 years ago

With the tooling, BYOB and CLI, we have to ensure our redirects are appropriately setup, also covering higher versions of what the runtime references (for minor-safe version ranges), so we can have that unification happen.

We can also have some logic as part of the resolution process to make sure those assemblies are resolved as a fallback, but this should only be required if we don’t feel the redirects aren’t sufficient.

MikeStall commented 7 years ago

By default, script runtime’s assembly resolution should unify SDK & Json.Net. These are special assemblies. We could emit a warning if this downgrades the user version, or allow an opt-out if this is truly a problem.

Our default logic hurts us in VS tooling and also commonly breaks BYOB:

  1. The default experience when a user adds a reference to JSON.Net, WebJobs, is to pull the latest. (Even though most often, the user doesn’t care about the exact version and just wants any working version )
  2. This commonly means they’ll pull a version greater than Script runtime’s referenced versions.
  3. Our default behavior in that case is to then load side-by-side (since we currently only do unification for lower versions), which will lead to failures since these assemblies expect to be unified.

In other words, our default behavior practically guarantees an error in the common customer case.

fabiocav commented 7 years ago

This would be a breaking change.

The unification may also be undesirable, and only applicable if those types are being used in parameter bindings, which is not always the case.

This would require an explicit user action if done, otherwise, existing customers would be impacted.

MikeStall commented 7 years ago
  1. This can be done in a non-breaking way. The presence of custom binding extensions provides that switch.
  2. The current behavior is most likely not what the customer wants.
  3. Trying to load multiple SDKs side-by-side (instead of unify) is absolutely incorrect and customers are not expecting that.
fabiocav commented 7 years ago
  1. The presence of a custom binding extension alone is not enough eliminate the risk of a breaking change. We'd need to unify on the actual bound type

  2. This is the desired behavior in many cases, we can't make this statement without actual data, but we do know that we have several scenarios where the current binding behavior is needed.

  3. Not absolutely incorrect in the context of functions, though. We do provide isolation and if customers are binding to Foo 3.0, I would argue the expectation is that Foo 3.0 is what they'd bind to at runtime (which is the current behavior). The unexpected behavior would be to automatically redirect to a different version because of an implementation detail of the runtime (for that to be expected, we'd require them to understand that)

fabiocav commented 7 years ago

Something else worth noting is that our set of dependencies with the VS tooling is controlled by us. We can absolutely restrict the dependencies to the supported versions which would give your consistency between what you compile, test and get when running in our runtime.