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

feat: lazy timer commands #242

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ericnorris
Copy link

Before this commit, it was not possible to lazily declare an fb_timers entry that utilized a node attribute inside the command.

After this commit, specifying a Proc for command or entries in commands will cause fb_timers to evaluate the proc when rendering the service template.

This is related to #233. I can't exactly recall where we landed on whether it was acceptable to add Proc support to cookbooks, but this particular example has come up enough times that I feel that it's worth enabling.

(I couldn't run these tests locally, so I might need to iterate a bit if I made a mistake)

Before this commit, it was not possible to lazily declare an fb_timers
entry that utilized a node attribute inside the command.

After this commit, specifying a Proc for `command` or entries in
`commands` will cause fb_timers to evaluate the proc when rendering the
service template.
Copy link
Collaborator

@jaymzh jaymzh left a comment

Choose a reason for hiding this comment

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

Yes, this is definitely the preferred way to do these things.

@ericnorris
Copy link
Author

Thanks @jaymzh! I think the kitchen test failures are unrelated? I believe the spec tests look good though.

@jaymzh
Copy link
Collaborator

jaymzh commented Nov 8, 2024

yeah kitchen tests have been broken a while, and no one has had time to go fix them.

@joshuamiller01 or @dafyddcrosby or someone will pull this in internally to run against extra internal tests and then merge if there's no issues.

ericnorris added a commit to etsy/chef-cookbooks that referenced this pull request Nov 11, 2024
@facebook-github-bot
Copy link
Contributor

@dafyddcrosby has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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.

3 participants