-
1. Re: Application Deployment Event Notification
kcbabo Jul 20, 2011 3:22 PM (in response to rcernich)At first I was like ... ehhh. Then I was like .... hmmm. Now, I think I'm like ... uh huh.
One thing that surprised me was the granularity in life cycle events. Do you intend to use that information in the admin stuff now or is this more of a future thing? Overall, I think this is promising.
-
2. Re: Application Deployment Event Notification
rcernich Jul 20, 2011 5:01 PM (in response to kcbabo)Hey Keith,
Thanks for the feedback.
At first I was like ... ehhh. Then I was like .... hmmm. Now, I think I'm like ... uh huh.
Sounds like some refinement is necessary. Could you elaborate, especially on the "ehhh" part? I added the "ArchiveName" field simply becasue name is not required in the SwitchYard schema, but is required for the admin application object. I'm not particularly proud of the way I wedged it in there and am not exactly sure if I fit it in correctly with the CDI deployer.
One thing that surprised me was the granularity in life cycle events. Do you intend to use that information in the admin stuff now or is this more of a future thing? Overall, I think this is promising.
I think initialized and destroyed are required, since the deployment basically maps to an admin application object. Those events let us know of an application becoming available and becoming unavailable, respectively.
The other events are simply thinking ahead. I could remove all the "pre" events and consolidate the failure events into something like:
- initialized
- started
- stopped
- destroyed
I changed the init, start, stop and deploy methods from abstract to final, adding abstract "do" methods for each to ensure the listeners were notified and managed appropriately (e.g. exception handling).
DeploymentEventNotifier is simply a "functional" interface used by notifyListeners() which loops over the listeners. (Scala has been seeping into my subconcious. )
Rob
-
3. Re: Application Deployment Event Notification
kcbabo Jul 20, 2011 8:14 PM (in response to rcernich)At first I was like ... ehhh. Then I was like .... hmmm. Now, I think I'm like ... uh huh.
Sounds like some refinement is necessary. Could you elaborate, especially on the "ehhh" part? I added the "ArchiveName" field simply becasue name is not required in the SwitchYard schema, but is required for the admin application object. I'm not particularly proud of the way I wedged it in there and am not exactly sure if I fit it in correctly with the CDI deployer.
The 'ehh, hmm, uh huh' stuff was just my lame attempt at humor. I was surprised at first to see the eventing stuff, but after I digested it, I think it might work well. My impression is that it will map cleanly to things like JMX notifications, which of course would be an admin implementation detail.
If application name is not required in the schema, I think defaulting to archive name is reasonable. I need to think a bit about whether application name should be required ... hmmm. Not sure we need to track archive name independently though vs. just setting it as application name. I'll take another look.
-
4. Re: Application Deployment Event Notification
rcernich Jul 21, 2011 10:18 AM (in response to kcbabo)The 'ehh, hmm, uh huh' stuff was just my lame attempt at humor. I was surprised at first to see the eventing stuff, but after I digested it, I think it might work well. My impression is that it will map cleanly to things like JMX notifications, which of course would be an admin implementation detail.
I think I missed the humor because deep down, I was like "ehh" myself (hence my opening it up for discussion). One thing that I don't like is that the configuration object is exposed through the interface by DeploymentListener.serviceDeployed(CompositeServiceModel).
If application name is not required in the schema, I think defaulting to archive name is reasonable. I need to think a bit about whether application name should be required ... hmmm. Not sure we need to track archive name independently though vs. just setting it as application name. I'll take another look.
I think the archive name will probably be needed (at some point) by the management tooling to allow operations like undeploy. I think I'll probably change the type from a QName to a simple String.
Thanks again for the feedback.
Rob
-
5. Re: Application Deployment Event Notification
rcernich Jul 24, 2011 11:32 AM (in response to rcernich)Here are the final changes: https://github.com/rcernich/core/commit/a6156ae89343f6ed0c94d6624d91bf421eb4aa20
The interesting classes are:
- DeploymentListener - the listener API.
- AbstractDeployment - the event generator.
- SwitchYardBuilder - DeploymentListener implementation which builds a SwitchYard model based on deployment events.
- SwitchYardDeployment - modified based on changes to AbstractDeployment. Specifically, these no longer extend Deployment, but wrap it, a la composition.
And the changes to the AS7 management extension which uses them: https://github.com/rcernich/core/commit/d248aab9a65600f8e9fbf916d24213f177d82a62
I'll create a separate thread to discuss the management operations.
-
6. Re: Application Deployment Event Notification
tfennelly Jul 25, 2011 6:23 AM (in response to rcernich)This small suggestion will prob go down like a lead balloon, but I'll fire it out there anyway
The DeploymentListener interface looks great and makes sense, but one small potential prob I see is that we might get stuck with a versioning issue on it i.e. once it becomes part of the public API it becomes difficult to extend/modify because doing so would lead to backward-compatibility issues. Of course, one could say this about any interface, but it would seem like these monitoring style interfaces would be prime candidates for extension, as we add more monitorable events etc.
One possible alternative approach would be to provide something like the @Observes model in CDI (the Arquillian SPI has something similar). Basically, listener impls would not implement any interface. Instead, they would just have event @Observes methods on them, with the events wrapped in Event objects. So... something like....
public class FunkyListener { public void observeInitializing(@Observes Initializing initializing) { AbstractDeployment deployment = initializing.getDeployment(); // etc... } public void observeInitialized(@Observes Initialized initialized) { AbstractDeployment deployment = initialized.getDeployment(); // etc... } // Etc... }
With this model, adding new @Observable events is trivial and would not break backward compatibility with existing listener impls out there. Of course, it also means the listener impls can be a bit cleaner because it means they only need to implement methods for events that they are interested in @Observing.