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

#775 - Explicitly resolve deferred values #789

Merged
merged 3 commits into from Nov 28, 2023

Conversation

fetzerms
Copy link
Contributor

@fetzerms fetzerms commented Jul 19, 2023

From Puppet Agent v8 onwards, a deferred value needs to be explicitly resolved.

Summary

This adds explicitly resolving of deferred values. See #775
If that fix is suitable (works for me), I'll be happy to add Spec and acceptance tests for deferred values.

Additional Context

I don't know of the root cause. Seems Puppet Agent v8 is doing things differently than <=v7

Related Issues (if any)

#775

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified. (For example puppet apply)

From Puppet Agent v8 onwards, a deferred value needs to be explicitly resolved.
@CLAassistant
Copy link

CLAassistant commented Jul 19, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM!

@smortex
Copy link
Collaborator

smortex commented Sep 26, 2023

Failed unit tests seems transient. Closing and re-opening to trigger a new run.

@fetzerms
Copy link
Contributor Author

fetzerms commented Oct 12, 2023

@smortex - As the build is green now, do we think we can merge this? I think unit tests are not possible/required for this change. But if they are, I'm happy to be guided on what to test :-)

@smortex
Copy link
Collaborator

smortex commented Oct 14, 2023

That seems good to me. There is no "policy" AFAIC for external contributors like me and other folks that where added as reviewers, but I would prefer to see a review/approval by someone from @puppetlabs since they own the project.

Copy link
Contributor

@joshcooper joshcooper left a comment

Choose a reason for hiding this comment

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

I filed a puppet bug about this, the intention was not to leak deferred values into custom type/providers. But this seems like an ok workaround.

@bastelfreak bastelfreak merged commit b355f42 into puppetlabs:main Nov 28, 2023
82 of 84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants