Sandbox importing

[VegetarianOrc]

What was changed

• Added new temporalio.worfklow.SandboxImportNotificationPolicy enum.
• Added new SandboxRestrictions.import_notification_policy setting to control the notification policy for imports processed by the sandbox.
• Add new context manager and accessor to workflow.unsafe to allow overriding the SandboxImportNotificationPolicy set on the configured SandboxRestritions.
• Added new RestrictionContext.in_activation field to indicate if the sandbox is handling an activation so we can differentiate between workflow load time.
• Updated _Importer to respect the override and configured policy to:
  1. Warn if an import occurred during activation.
  2. Warn if an import occurs that was not intentionally passed through to the sandbox.
  3. Raise an error if an import occurs that was not intentionally passed through to the sandbox.
• Updated SandboxRestrictions.default to use SandboxImportNotificationPolicy.WARN_ON_DYNAMIC_IMPORT
• Added a few combinations of SandboxImportNotificationPolicy flags to SandboxRestrictions for ease of use.

Why?

The introduction of the new import notification policy aims to tackle two challenges SDK users face that are described in #790. The new behaviors can be combined to provide the desired level of verbosity.

Increase Visibility into Dynamically Imported Modules

Some libraries will import modules as needed or as a result of a function call. In these cases, it may not be obvious to users that this is occurring and can cause unnecessary memory overhead. This new default setting will warn any time an import occurs during workflow task execution. Users may opt out of this warning by configuring the SandboxRunner with a SandboxImportNotificationPolicy or by using the context manager workflow.unsafe.sandbox_import_notification_policy.

Increase Discoverability of Modules Requiring Passthrough

Some users prefer to ensure that imports used by the sandbox are explicitly passed through. The new opt-in settings of SandboxImportNotificationPolicy.WARN_ON_UNINTENTIONAL_PASSTHROUGH and SandboxImportNotificationPolicy.RAISE_ON_UNINTENTIONAL_PASSTHROUGH allow users to discover any import that occurs in the sandbox that is not explicitly passed through. These settings are mutually exclusive and if both are set on the import notification policy, RAISE_ON_UNINTENTIONAL_PASSTHROUGH will be respected. Users may opt into this behavior by configuring the SandboxRunner with a SandboxImportNotificationPolicy or by using the context manager workflow.unsafe.sandbox_import_notification_policy.

Checklist

Closes #790

How was this tested:

New tests were added to exercise a variety of policy combinations and verify that code run in an interceptor will also respect the new setting.

Any docs updates needed?

• The Python SDK sandbox docs should be updated with information about the new default warning, other options, and examples of how to adjust the setting.

[cretz]

Looks great, just minor naming/default things

[cretz]

Hrmm, remind me, did we decide not to have a kind of with workflow.intentional_import_reload(): type of thing yet? I know you can do it here with the policy altering, but I think I remember we decided to not do the nicer sugared form at this time, just confirming.

[VegetarianOrc]

My thought here was that this could be combined with the pre-defined policies to achieve a similar effect. It's a bit more wordy but feels okay to me to use something like

with workflow.unsafe.sandbox_import_notification_policy(
    SandboxRestrictions.import_notification_policy_silent
):
    #...

That being said, I think it makes just as much sense to have a method like you're suggesting that applies the correct policy. I put a little time into thinking about this before opening the PR but couldn't settle on any wording that would make the behavior being applied immediately obvious. Very happy to add any options you'd recommend and/or remove this one.

[cretz]

I think we can leave off a with workflow.intentional_import_reload() helper for now, mainly because I don't think it'd be common

[VegetarianOrc]

Sounds good. Along these same lines, do you still think it's worth removing the various static helpers per your other comment? I think either way makes sense. IMO they're helpful, but as you pointed out are pretty straightforward to reason about even without them.

[cretz]

I do think it's still worth removing them. Having static, wordy helper constants just to help you set an enum flag seems a bit off. The ergonomics of setting an enum flag are good enough on their own IMO.

[cretz]

I think we should set the default here, it's a backwards incompatible change to add a new required field here (and then not set it in our tests in the global/general area)

[VegetarianOrc]

Good catch. Making this change now.