brooklyncentral / brooklyn

This project has moved and is now part of the ASF
https://github.com/apache/incubator-brooklyn
72 stars 27 forks source link

EntityProxy.invoke should check that the task context matches target entity #1370

Open sjcorbett opened 10 years ago

sjcorbett commented 10 years ago

.. and should log a warning or throw an error if it does not.

This came from this scenario:

Which is a confusing error.

Issue prompted by this conversation on IRC:

[12:19:21]  <samc_>  alexheneveld: a question on task queuing contexts for you. I have a Group that invokes a method X on its members. X restarts the member. the restart eventually defers to SoftwareProcessDriverLifecycleEffectorTasks.restart(), which calls entity(), which is defined as:
[12:19:21]  <samc_>  (EntityInternal) BrooklynTaskTags.getTargetOrContextEntity(Tasks.current())
[12:19:36]  <samc_>  but in this case the current task is the Group, not the entity
[12:20:32]  <samc_>  should the method X set the task queueing context before running the restart?
[12:21:01]  <samc_>  at the moment it throws a ClassCastException: Group cannot be cast to SoftwareProcessImpl
[12:21:57]  <alexheneveld>   samc_ - direct cross-entity method calls are dangerous as they will break if the entity is remoted
[12:22:17]  <samc_>  so I should invoke an effector instead?
[12:22:19]  <alexheneveld>   consider them supported only for effectors
[12:22:43]  <alexheneveld>   if you make X an effector then the Proxy behaviour should automatically set up a task when you invoke method X
[12:25:26]  <alexheneveld>   we should have a check in EntityProxy.invoke that checks if there is a Tasks.current() with an entity context set, that that entity context matches the target entity and logs warning (or throws) if not, citing the reason above.
sjcorbett commented 10 years ago

@aledsage Do you agree with the comments above?