Skip to content

Conversation

@sjcorbett
Copy link
Contributor

Resolves useful parameters of implementations of the AddEffector and AddSensor entity initializers, meaning that their values can be sourced from DslComponents.

Affects:

  • SshCommandEffector's command and executionDir
  • SshCommandSensor's command and executionDir
  • CreatePasswordSensor's passwordLength and acceptableChars
  • JmxAttributeSensor's objectName, attribute and defaultValue

Means blueprints can use DslComponents in their definition.
super.apply(entity);

Integer passwordLength = EntityInitializers.resolve(params, PASSWORD_LENGTH);
String acceptableChars = EntityInitializers.resolve(params, ACCEPTABLE_CHARS);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply is called before managing any of the entities in the spec hierarchy (i.e. a camp plan) so this can't be resolved at this point. It would just block app creation. Could you add some tests to prove me wrong :).
StaticSensor does the right thing by pushing all of it in tasks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, this was lazy of me.

@grkvlt
Copy link
Member

grkvlt commented May 24, 2016

@sjcorbett One thing you should probably do here is move AddSensor to a different package, perhaps org.apache.brooklyn.core.sensor instead? Also, would there be any benefit gained by making the initializers be EntityAdjunct objects, supporting configuration properly etc.

@sjcorbett
Copy link
Contributor Author

@grkvlt @neykov Thanks for your comments. This whole approach is flawed because it doesn't survive rebind. Entity initialisers have been written to fire once, which is unfortunate because it's counterintuitive when used alongside everything else in a blueprint. I will explore the suggestion of using EntityAdjuncts.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants