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

Make entity_uuid work for Chef Infra 15 #371

Merged
merged 1 commit into from
May 31, 2019
Merged

Conversation

alexpop
Copy link
Contributor

@alexpop alexpop commented May 31, 2019

Fixes #368

@alexpop alexpop requested a review from a team May 31, 2019 09:12
Signed-off-by: Alex Pop <alexpop@users.noreply.github.com>
@alexpop alexpop force-pushed the ap/node-uuid-for-chef15 branch from 5570edd to 18c0ddf Compare May 31, 2019 09:36
@lamont-granquist
Copy link
Contributor

I dunno maybe it doesn't matter but this module gets included in a lot of different contexts where it looks like there my be no node object so that will blow up. It seems to be used off of a run_status which seems to be the actual Chef::RunStatus object? I don't understand how the wiring happens there. It does inject itself into the Chef::Recipe DSL, but that shouldn't appear in the RunStatus object which inherits from nothing and AFAIK nothing gets mixed into it.

So.... this might work, but there still may be a pile of software developer footguns left lying around on the floor here...

This also clearly broke, but no tests went red. That indicates that there really needs to be a test.

@alexpop
Copy link
Contributor Author

alexpop commented May 31, 2019

I would like to see a refactoring on how the node guid is identified.
For now, let's get this in to unblock Chef Infra 15 users. Travis tests are passing and I also manually tested it with the chef-automate reporter.

@alexpop alexpop merged commit 0b32136 into master May 31, 2019
@lamont-granquist lamont-granquist deleted the ap/node-uuid-for-chef15 branch June 3, 2019 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Cookbook broken with Chef-15
3 participants