Skip to content
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

Allow Python dynamic provider resources to be constructed outside of __main__ #7755

Merged
merged 3 commits into from
Aug 13, 2021

Conversation

lukehoban
Copy link
Contributor

@lukehoban lukehoban commented Aug 12, 2021

The underlying library dill that we use for serializing dynamic providers into Pulumi state for Python dynamic providers serializes classes differently depending on whether they are in __main__ or in another module. We need the by-value serialization to be applied in all cases.

uqfoundation/dill#424 is tracking adding the ability into dill to specify this by-value serialization explicitly, but until then, we will temporarily re-write the __module__ of a provider class prior to serialization, so that dill behaves as we need for the dynamic provider use case.

Fixes #7453.

…`__main__`

The underlying library `dill` that we use for serializing dynamic providers into Pulumi state for Python dynamic providers serializes classes differently depending on whether they are in `__main__` or in another module.  We need the by-value serialization to be applied in all cases.

uqfoundation/dill#424 is tracking adding the ability into `dill` to specify this by-value serialization explicitly, but until then, we will temporarily re-write the `__module__` of a provder class prior to serialization, so that `dill` behaves as we need for the dynamic provider use case.

Fixes #7453.
@lukehoban lukehoban requested review from justinvp and t0yv0 August 12, 2021 20:43
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@benesch
Copy link
Contributor

benesch commented Aug 12, 2021

This is wonderful! Thanks very much, @lukehoban!

@emiliza emiliza modified the milestone: 0.60 Aug 12, 2021
@lukehoban lukehoban merged commit ebb0e6a into master Aug 13, 2021
@pulumi-bot pulumi-bot deleted the lukehoban/7453 branch August 13, 2021 03:02
lukehoban pushed a commit that referenced this pull request Sep 1, 2021
…side of `__main__` (#7755)"

This reverts commit ebb0e6a.

The changes in #7755 introduced a regression tracked in #7795.  It is not yet clear how to retain the desired behaviour introduced in #7755 while avoiding this regression, so for now we will revert those changes, and re-open #7453 to track a deeper fix.  That fix may require making changes to upstream `dill` to properly support these serialization requirements.

Fixes #7795.
lukehoban pushed a commit that referenced this pull request Sep 2, 2021
…side of `__main__` (#7755)" (#7889)

This reverts commit ebb0e6a.

The changes in #7755 introduced a regression tracked in #7795.  It is not yet clear how to retain the desired behaviour introduced in #7755 while avoiding this regression, so for now we will revert those changes, and re-open #7453 to track a deeper fix.  That fix may require making changes to upstream `dill` to properly support these serialization requirements.

Fixes #7795.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic resources in Python don't work outside of __main__
5 participants