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

New Resource interface #1212

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

krimdomu
Copy link
Contributor

Hello,

this is the new resource interface based on Moose and Roles.

Things the new interface adds (or helps with):

  • export of resources to main namespace (or not)
  • easy provider multiple kinds of the same resource (no code needed to test which parameter was set and which not, Rex::MultiSub handles it)
  • validation of parameter list. it uses MooseX::Params::Validate to validate parameters. So it is possible to validate against all kinds of variable/object types. and also easily provide defaults.
  • usage of Moose-Roles so that the provider of a resource (the thing which does the real work) can tell rex which ensure options it can handle (possibility to have sane error messages)
  • common interface for things like: on_change, timeout, only_if, on_, before_, after_*, ... (meta resource parameters)
  • every resource is easily extendable with providers. for example the file() resource has a POSIX provider to write files on linux/unix like filesystems. If you want to write files on webdav you just need to implement a provider and append it to the resource call in the Rexfile.
  • better reporting facilities (the reporting is not handled by the Resource)

Feel free to comment.

If you want to see more examples, you'll find some pocs on my private repo (run, file, pkg, kernel):
https://github.com/krimdomu/Rex/tree/refactor/resource-base/lib/Rex/Resource

Currently it only has one downside: It doesn't run on Windows. Windows can't handle timeouts. (Which also doesn't work with the current version, but i use the module Time::Out and because of this it doesn't start on windows)

@krimdomu krimdomu added the enhancement An idea to improve existing behavior label Jan 19, 2019
@krimdomu krimdomu added this to the 1.7.0 milestone Jan 23, 2019
@krimdomu krimdomu force-pushed the refactor/resource-interface branch from 5ac4f80 to 2286e33 Compare June 29, 2019 05:06
@ferki
Copy link
Member

ferki commented Jul 22, 2019

@krimdomu: I've done some initial review, and I'd like to provide you some feedback before it gets merged :)

  1. It currently fails t/resources.t and t/param_lookup_resource.t tests. I didn't take a closer look yet, but this obvisously needs to be fixed.
  2. I find the whole change too big to easily comprehend for a full review. Do you think it would be possible to split it up into a few smaller changesets instead? Perhaps one PR could deal with the changes to the resource interface, and another one with migrating the firewall resource (and/or more resources).
  3. Are those changes backwards-compatible (is it a second interface next to the existing one, or replacing that)? Does it break any of the existing resource implementations?

@ferki ferki removed this from the 1.7.0 milestone Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An idea to improve existing behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants