| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
|
|
|
|
|
|
| |
When we introduced verification of options, we forgot to handle the case that
global options from the Puppet settings system could be passed to the face.
This, in turn, means that the system would fail if you used any of those.
This remediates that, and now these work as expected.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
| |
Rewrite the process of validating and updating the options to fully reflect
the contract - we fail if there are unknown options passed, report nicely
errors of duplicate names, pass only the canonical names to the action code,
and generally enforce nicely.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Rather than having multiple, separate operations that modify and validate the
arguments to an action, a single pass makes sense. This also means less walks
across the set of data, and a few less expensive method calls in Ruby.
Additionally, we work on a duplicate of the arguments hash rather than
directly modifying the original. Because everything we do is at the top level
key/value mapping, this is sufficient to isolate the original.
While mostly theoretical, we now don't mutilate the hash passed in, so the
user won't get nastily surprised by the fact that we could have done so.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Part of the "social contract" of Faces, Actions and Options is that the
metadata we collect is authoritative: it covers everything that is possible.
In the initial release we didn't enforce that around options. If you passed
an unknown option in the hash, we just silently ignored it in validation and
made it available down in the action.
Now, instead, we enforce that rule. If you pass an unknown option we raise an
error and complain; anything that gets to the action will be listed in the set
of inspectable options.
Cases that depended on this behaviour to pass arbitrary content in the hash
should be rewritten to move that content down a level: take a hash value for
one option, and use that for your free content.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we invoke an action, we parse a set of options. These have a canonical
name, and optionally a set of aliases. For example, :bar might have :b as an
alias to allow a short name to be given.
Previously we would just pass this on as received; if you passed :bar you got
:bar, and if you passed :b you got :b. This works, but means that every
action has to write the same code to extract the appropriate version of an
option from whatever set of aliases might be passed.
Now, instead, we centralize that and always pass options as their canonical
name to the action code. This makes it simpler to work with. (This happens
before any validation, or other user-supplied, code to simplify everything
that handles options.)
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
| |
When we define an action on an older version of a Face, we must be sure to
directly load the core of that version, not just define it with the external
Action(s) that it had.
Otherwise we break our contract, which is that any core Actions for a specific
version will be available to your external Action for as long as we support
that core version.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we first touch a Face, we load all the available Actions from disk.
Given they define themselves against a specific version of a Face, they are
automatically available tied to the correct version; this makes it trivially
possible to locate those on demand and return them.
Now, we have the ability to find and, consequently, invoke Actions on older
versions of Faces. We don't load enough context, though: the older face will
only have external Actions defined, not anything core.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
| |
Given the inheritance model for actions, we are sometimes going to need to set
them to 'default' at runtime, rather than during their static declaration.
Add tests to verify that this works correctly, and update the code to ensure
that happens. This gives up caching of the default action, but this should be
an extremely rare operation - pretty much only CLI invocation, really.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
| |
This implement support for options with default values, allowing faces to set
those values when not invoked. This can eliminate substantial duplicate code
from actions, especially when there are face-level options in use.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
| |
This introduces a class representing a semantic version, and
implementing a few of the most common uses of them: validation,
comparison, and finding the greatest available version matching
a range. This refactoring also allows us to easily expand our
matching of version ranges in the future, which is a big plus.
Reviewed-By: Daniel Pittman
|
| |
|
|
|
|
|
|
| |
The problem was caused by the fact that the
options method returns a list of options that
treated the aliases as seperate options.
The fix is to only maintain a list of options
and not add all aliases to the options list.
|
| |
|
|
|
|
|
| |
2.7.x"
This reverts commit b7ee0258ab40478329c20177eda9b250f27ede18, reversing
changes made to 8fe2e555ac3d57f5b6503ffe1a5466db8d6e190a.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
puppet help was reprinting every option once
for every alias that is had.
This fix involves only storing the option.name
in the @options instance var for both face and
actions options. The @options_hash still
maintains the list of options and aliases as its
keys.
Reviewed-by: Daniel Pittman (puppet-dev list)
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We accidentally omitted whitespace between multiple options while building the
synopsis. This fixes that, by introducing a breakable space in the right
location.
Additionally, we extract the code that was 99 percent identical from the face
and action synopsis generators, push it down into the documentation module,
and then invoke it from both places.
This eliminates the duplicate code, allowing me to fix that bug once and have
it apply to both parts of the code; this is pretty much assured to be true
any time we change the synopsis generation.
Reviewed-By: Nick Fagerlund <nick.fagerlund@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
We earlier moved to duplicating Action objects in the Faces subsystem to
ensure they had the correct binding context during execution and introspection.
This was correct, but introduced a bug where we would report both the parent
and child binding as separate entries with duplicate names, in the list of
actions.
This flowed on to the help output, where it would cause every inherited action
to be listed twice: once on the parent, once on the child. (This was actually
worse if the inheritance was deeper: we would duplicate once for every level
between the instance and the origin of the action.)
|
| |
|
|
|
|
|
|
| |
Ruby 1.9 is strict about argument arity for methods that are
metaprogrammatically defined. A ton of specs that were setting up
when_invoked didn't pass options even though they should have been.
Reviewed-by: Daniel Pittman <daniel@puppetlabs.com>
|
| |
|
|
|
|
|
| |
Since we never shipped this in a real release, we don't need to maintain
compatibility. So, remove it entirely from the codebase.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |
|
|
|
|
|
|
| |
Where we need special support for :for_humans as an alias for :console, call
it out in comments. This makes it clear to someone who wonders why what the
actual underlying purpose of the whole thing is.
Reviewed-By: Jacob Helwig <jacob@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
| |
When we hit a syntax error in any face, a whole bunch of unrelated face things
would blow up in horrible ways. Stack traces for all...
Now, instead, we catch that fault but specifically only in the face file
and report it through our error logs, then quietly ignore the face.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When we query for all faces we need to scan the entire Ruby load path, look
for everything that looks like a face, and load it up. That is a fairly
expensive operation, especially on a platform that has slow I/O.
*cough* EC2 *cough*
Because of that we only scan once, and assume that the list is static
thereafter; this works OK out in the field, but sucks in testing where that
global state gets in the way of the rest of our fiddling under the hood.
This resets the '@loaded' member of the collection additionally, which is what
should be done since we have reset the rest of the collection at the same
time.
We don't bother to reset it, as an extra scan during tests is not a problem.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
| |\
| |
| |
| | |
2.7.x
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We used to let actions be declared without the `when_invoked` block, which was
usually a sign of either someone writing their method code direct in action
declaration, or someone forgetting to add their code at all.
This was just let silently by: the error only showed up when you finally tried
to invoke the action, and a NoMethod error was raised by the face.
...except for our own testing. We took advantage of this a whole pile of
times in there; fixing the original UI issue means fixing all those too.
Reviewed-By: Nick Lewis <nick@puppetlabs.com>
|
| |/
|
|
| |
Reviewed-By: Daniel Pittman
|
| |\ |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`before_action` decorations should always resolve in resolution order
from most general (inherited from furthest away) to most specific
(declared on the instance), and should always execute Face-level
option decorations before action-level option decorations.
`after_action` decorations should execute in the opposite order.
Reviewed-By: Daniel Pittman
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
From the command line, and other facades, it is fairly difficult to call an
action with two aliases of the same option in the invocation, but from the
Ruby API it would be relatively easy to achieve.
This is now checked, and raises an error, so that we don't accidentally have
strange or unusual behaviour coming out of the whole system.
Reviewed-By: Pieter van de Bruggen <pieter@puppetlabs.com>
|
| |\ \
| |/
|/| |
|
| | |
| |
| |
| |
| |
| |
| | |
This extends the last of the documentation support, down into options, so they
can be described as expected. In the process we split out the modular docs
API into a full and short version options only want short docs, but the
behaviours are identical to the full version.
|
| |/
|
|
|
|
| |
This allows users to write before_action advice that does basic
option validation very easily.
Reviewed-By: Daniel Pittman
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
"Invisible glob", or "prefix", version matching means that when you specify a
version string to use you can specify as little as one version number out of
the semantic versioning spec.
Matching is done on the prefix; an omitted number is treated as "anything" in
that slot, and we return the highest matching versioned face by that spec.
For example, given the set of versions: 1.0.0, 1.0.1, 1.1.0, 1.1.1, 2.0.0
The following would be matched:
input matched
1 1.1.1
1.0 1.0.1
1.0.1 1.0.1
1.0.2 fail - no match
1.1 1.1.1
1.1.1 1.1.1
1.2 fail - no match
|
| |
|
|
|
| |
This rewrites a block of identical tests down to a little table, then applies
the test over that.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We previously used *args to collect all arguments to the action when_invoked
block, then tried vaguely to massage some little bits of them into the right
shape.
Methods defined with blocks, in Ruby 1.8, also have some fun behaviours. The
most special is that if you pass more than one argument to a block defined
with only one Ruby will automatically coerce the arguments into an array – and
this is preserved when it is bound to a method.
This led to routine situations where we would pass the wrong number of
arguments to the block because, say, the user gave an extra argument on the
command line.
Instead of failing this would transmogrify the arguments in counterintuitive
ways, and end up with horrible stack traces when that interacted badly with
the code as written.
Now, instead, we work out the right argument format based on the arguments
that the when_invoked block takes. This gives much better (albeit perhaps not
so user friendly) behaviour at the interface level. Which is, at least,
consistent with other Ruby API.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |\
| |
| |
| | |
Fix the conflicts over changes in my previous commit.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
We no longer establish the rendering mode in the actions; they just default to
"nothing", and let that flow on out to the application layer. That lets the
facade we put before the face determine the default behaviour.
This is mostly a no-op down in the CLI side, but it makes it much easier to
integrate into MCollective, HTTP-API, and for other non-CLI users of Faces.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| | |
We can return a method bound to the current face instance when we access the
'when_rendering' hook, which allows us to directly call them. Make that
change, and add appropriate testing.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
These define the API used by folks writing actions that supports their
rendering hooks. 'when_rendering' defines a helper method on the interface,
which runs the users code in their expected context.
'render_as' just sets the default rendering format; by default this is
:for_humans.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| | |
We tested that we could make something a default action, but we also want to
assert that we never accidentally make everything the default action.
Paired-With: Max Martin <max@puppetlabs.com>
|
| |\ \
| |/
|/| |
|
| | |
| |
| |
| |
| |
| |
| | |
This adds the 'description' method to the faces and actions, as well as
structured testing to ensure that the DSL works as expected.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
Our summary documentation is used to provide single-line context to faces,
actions, and other items. To support this we hard-fail if someone tries to
use the summary to embed the long documentation, and point them to the right
place to add the extended text.
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| | |
Running spec/unit/option_spec.rb before requiring puppet/interface
caused a circular require, and a failure.
Paired-With: Max Martin
|
| |/
|
|
|
|
|
|
| |
This adds another hook into the generated wrapper, which invokes a method to
validate arguments. This is used to raise an exception when required options
have not been passed to the method.
Reviewed-By: Daniel Pittman <daniel@puppetlabs.com>
|
| |\
| |
| |
| | |
'feature/2.7.x/6978-face-and-action-options-should-have-hooks-for-various-actions' into 2.7.x
|
| | |
| |
| |
| |
| |
| |
| |
| | |
We require that hooks take exactly three arguments; now we enforce that in the
DSL, to ensure we give good, and early, errors to users who do the wrong
thing.
Paired-With: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Options can now add before_action and after_action blocks; these are invoked
before or after any action is invoked on the face. This allows these options
to declare common behaviour and have it automatically applied to the actions
invoked.
Option hooks have no defined order of invocation: they will run in a
completely random order. Where there are dependencies they should be on the
value of the options hash passed to the invocation, not on side-effects of the
other invocations.
You are not able to influence the arguments, options, or calling of the action
body in a before or after decorator. This is by design.
The invocation passes to the hook:
1. The action object representing this action.
2. The arguments to the action, as an array.
3. The options for the action, as a hash.
Paired-With: Max Martin <max@puppetlabs.com>
|
| | |
| |
| |
| | |
Reviewed-By: Max Martin <max@puppetlabs.com>
|
| |/
|
|
|
|
|
| |
This also enables the 'help' action on the 'help'
face to serve as a default action.
Reviewed-By: Daniel Pittman
Reviewed-By: Nick Lewis
|
| |
|
|
|
|
|
| |
We now use a shebang of: #!/usr/bin/env rspec
This enables the direct execution of spec tests again, which was lost earlier
during the transition to more directly using the rspec2 runtime environment.
|
| |
|
|
|
|
|
|
|
| |
After some discussion we decided that most uses of the Puppet Face
infrastructure were about single faces on their own, not about the collection,
and so we were better referring to Puppet::Face[...] in code.
This implements that by translating names and references in the Ruby code to
the new, s-less, name.
|
| |
|
|
|
|
|
| |
This extends the summary function down through the actions themselves,
allowing us to display a useful summary to the user.
Reviewed-By: Matt Robinson <matt@puppetlabs.com>
|
