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

Fill in Parameterization Value #2202

Merged
merged 1 commit into from
Jul 19, 2024
Merged

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Jul 19, 2024

This commit adds support for the new .Parameterization field on the schema. As part of
doing so, I have moved parsing parameterization into it's own package.

Fixes #2149

@iwahbe iwahbe requested review from guineveresaenger and a team July 19, 2024 02:17
@iwahbe iwahbe self-assigned this Jul 19, 2024
Copy link

codecov bot commented Jul 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.62%. Comparing base (11e23df) to head (cfd70ae).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2202      +/-   ##
==========================================
- Coverage   61.30%   60.62%   -0.69%     
==========================================
  Files         353      354       +1     
  Lines       38512    46297    +7785     
==========================================
+ Hits        23611    28068    +4457     
- Misses      13354    16674    +3320     
- Partials     1547     1555       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@mjeffryes mjeffryes left a comment

Choose a reason for hiding this comment

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

Could you update the description on the PR to actually describe the change before committing so we get a more useful commit message in the git history?

Comment on lines +92 to +95
// IntoArgs converts a [Value] into an [Args].
//
// We can do this because [Value] is a fully resolved [Args], and so it is always possible to
// go from [Value] to [Args].
Copy link
Member

Choose a reason for hiding this comment

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

nit. I assume the reverse is true as well and we should probably have testing that validates the round trip

Copy link
Member Author

Choose a reason for hiding this comment

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

The reverse is not true without contacting a registry and resolving the provider. Value is resolved, so it will look like this:

Value{Remote: &RemoteValue{
    URL: "registry.opentofu.org/azure/alz",
    Version: "0.11.1",
}}

There are many possible Args that could resolve into Value:

nameOnly := Args{Remote: &RemoteArgs{
    Name: "azure/alz",
}}
versionRange := Value{Remote: &RemoteValue{
    URL: "azure/alz",
    Version: "<=0.12.0",
}}
fullySpecified := Value{Remote: &RemoteValue{
    URL: "registry.opentofu.org/azure/alz",
    Version: "0.11.1",
}}

For a given Value, it can always be converted into a fully resolved Args value.

Copy link
Member

Choose a reason for hiding this comment

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

ah thanks for clarifying. (Sounds like there's still a possible round trip test, but only values -> args -> values, not args -> values -> args)

@iwahbe iwahbe force-pushed the iwahbe/parameterization-value branch 3 times, most recently from 85ed620 to 786b508 Compare July 19, 2024 15:54
This commit adds support for the new `.Parameterization` field on the schema. As part of
doing so, I have moved parsing parameterization into it's own package.

Fixes #2149
@iwahbe iwahbe force-pushed the iwahbe/parameterization-value branch from 786b508 to cfd70ae Compare July 19, 2024 18:00
@iwahbe iwahbe merged commit 4fbc7ed into master Jul 19, 2024
11 checks passed
@iwahbe iwahbe deleted the iwahbe/parameterization-value branch July 19, 2024 20:03
@mjeffryes mjeffryes added this to the 0.107 milestone Jul 24, 2024
@pulumi-bot
Copy link
Contributor

This PR has been shipped in release v3.88.0.

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.

Parameterized providers: support SDK parameterization
3 participants