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

Robust Attribute Tracing Support #75

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@clintoncwolfe
Copy link

clintoncwolfe commented Dec 3, 2014

No description provided.


There are some spec tests included in https://github.com/opscode/chef/pull/1373, but
that may not be what people want; and it does not address the existence of an
real Observable interface.

This comment has been minimized.

@lamont-granquist

lamont-granquist Dec 4, 2014

Contributor

I think I'm mostly concerned that the existing Node::Attribute implementation is turning into a mess, and the Chef 12 changes make for even more entry points to mutate things, and I'm not wild about the way everything is being hacked up with additional state coming along for the ride on the VividMash's and stuff. You found stuff like the difference between the AttrArray and the VividMashes when it comes to auto-promotion of the types and a lot of other edge cases like that. The caching stuff I bolted into Chef-12 to try to make it a bit faster are particularly ugly. So there's that at the lowest level. I'm not even super worried about having a proper Observable interface at this point, but the whole rest of the attribute implementation is lurching along and showing its age. I think I've got a design for how to do it all much better, though, and I think attribute tracing would slot on top of it pretty nicely and we could wrap all the mutators fairly easily.

Getting the information injected correctly is a bit more problematic. The use of class variables and global state will cause issues and that should get moved to the run_context or a proper Singleton class somewhere. Then I'm just not super wild about wrapping all the things that might set a node attribute in Chef::Node::Attribute.with_tracer_hint calls (but I don't have a better solution).

So I think to begin with it'd be nice to see a cleaner implementation of how Chef::Node and friends would be able to ask for the current 'context'. Ideally this goes into something like Chef::ExecutionTracer which is a Singleton with state on the current cookbook, file and such, and which Chef::Node can ask for the current cookbook, file and line number. We also may be able to reuse that code in the error inspectors as well. That would let you do a new feature as a new object, pretty much clean room, would have good separation of concerns and would be reusable outside of attributes.

Should probably come up with a better name than Chef::ExecutionTracker tho....

@nathenharvey

This comment has been minimized.

Copy link
Contributor

nathenharvey commented Mar 13, 2015

should we close this for now and revisit after Attributes API 2 is done?

@lamont-granquist

This comment has been minimized.

Copy link
Contributor

lamont-granquist commented Mar 13, 2015

yeah, we also need Attributes Core 2 as well

@clintoncwolfe

This comment has been minimized.

Copy link

clintoncwolfe commented Mar 13, 2015

WFM
On Mar 12, 2015 11:07 PM, "Lamont Granquist" notifications@github.com
wrote:

yeah, we also need Attributes Core 2 as well


Reply to this email directly or view it on GitHub
#75 (comment).

@nathenharvey

This comment has been minimized.

Copy link
Contributor

nathenharvey commented Mar 13, 2015

Closing for now. Will revisit or incorporate into the new attributes work that's being proposed.

@vinyar

This comment has been minimized.

Copy link

vinyar commented Apr 14, 2015

👍 for whenever this gets reopened

@odcinek

This comment has been minimized.

Copy link

odcinek commented Jul 24, 2015

👍 on this, would be great to revive chef/chef#1373

@lamont-granquist

This comment has been minimized.

Copy link
Contributor

lamont-granquist commented Jul 25, 2015

I need to ship the node attributes re-write when I get back off of vacation. Attribute tracing should get easier after I do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment