Enable catalog access feature #284
Conversation
kreeuwijk
changed the title
There was a problem hiding this comment.
On preliminary testing, the class-level context doesn't have access to the catalog, but is used during instances for calling get. This makes me think that I'm not fully understanding the object lifecycle around this, making me afraid of either running into issues where the catalog is not yet available (as the unit tests flag up) or the catalog leaks through the agent process across multiple invocations.
This is also limited in that the catalog is not available during puppet resource calls. While providers need to be prepared to handle this (and we need to call it out in docs), I don't think there's any way around this, as puppet resource just doesn't have a catalog.
Overall this will need a bit more investigation/exploration/change before we can merge it.
This commit updates the BaseContext class initialization method to take a second argument, catalog (default to nil), which ensures that the @catalog instance variable is populated with the value passed. It also updates the context method in resource_api to pass the catalog to that initializer if specified, or pass nil if not. This does not add unit or acceptance tests for the behavior.
michaeltlombardi
force-pushed
the
catalog_access
branch
from
9baf31c
to
d05f624
Compare
|
@kreeuwijk I did a minor fixup to get existing tests passing and manually validated that things are working, but I think we'll need some more extensive testing (acceptance and unit alike) and noodling out to be sure there's no weird side effects or changing during a run. |
|
Testing this might be a bit tricky as this will require a running puppet agent service and then verifying that the catalog from the first run doesn't leak into the subsequent runs. Not really sure how to set this up properly. |
|
With the inability to add automated tests at the moment, the risk of adding a new API surface that exposes an advanced raw access to the catalog which is contrary to the purpose of this simpler API, and the uncertainty of the clear purpose/need for the feature I am closing this PR. Thank you for your contribution, if you feel this should be included please reach out to myself and @binford2k |
This PR enables setting a
raw_catalog_accessfeature flag on the type, which then provides access to the catalog in the provider by usingcontext.catalog.