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

[codegen/python] fix deprecation warning on default values #13890

Merged
merged 1 commit into from Sep 12, 2023

Conversation

dixler
Copy link
Contributor

@dixler dixler commented Sep 6, 2023

Description

Removes Resource constructor deprecation checks in favor of deprecation checks in the ResourceArgs constructor.

Checks in the ResourceArgs constructor which will be changed to ResourceArgs.__configure__ in #13825 and compatible with dicts and objects that implement __getitem__ __setitem__.

Merge #13825 after and update __internal_init__ logic to use the ResourceArgs.__configure__

Fixes #12546

Checklist

  • I have run make tidy to update any new dependencies
  • I have run make lint to verify my code passes the lint check
    • I have formatted my code using gofumpt
  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@dixler dixler changed the title Dixler/12546/deprecated 2 [codegen/python] fix deprecation warning on default values Sep 6, 2023
@pulumi-bot
Copy link
Contributor

pulumi-bot commented Sep 6, 2023

Changelog

[uncommitted] (2023-09-12)

Bug Fixes

  • [programgen/python] Fix deprecation warning triggering on ResourceArgs with default values.
    #13890

@dixler dixler marked this pull request as ready for review September 7, 2023 14:57
@dixler dixler requested review from justinvp, Frassle and a team September 7, 2023 15:01
@dixler dixler requested a review from Frassle September 8, 2023 16:41
Comment on lines 38 to 43
"""
new_resource mocks resource construction calls. This function should return the physical identifier and the output properties
for the resource being constructed.

:param MockResourceArgs args.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"""
new_resource mocks resource construction calls. This function should return the physical identifier and the output properties
for the resource being constructed.
:param MockResourceArgs args.
"""

No need for doc comments about the abstract method in test code.

@pulumi.runtime.test
def test_func_with_default_value(my_mocks):
with patch("warnings.warn") as mock_warn:
pulumi_plant.tree.v1.RubberTree("my-tree", pulumi_plant.tree.v1.RubberTreeArgs())
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't totally obvious what this was testing to start, I would add a doc comment to this method saying what we're doing "constructing a resource with deprecated but default values and checking the default values don't trigger a deprecation warning"

Removes Resource constructor deprecation checks in favor of deprecation
checks in the ResourceArgs constructor. The Resource constructor would
always see the default value as set and would trigger deprecation
warnings if they exist.

Fixes #12546
@dixler dixler added this pull request to the merge queue Sep 12, 2023
Merged via the queue into master with commit c340723 Sep 12, 2023
50 checks passed
@dixler dixler deleted the dixler/12546/deprecated-2 branch September 12, 2023 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants