Esri / activemq-for-geoevent

ArcGIS GeoEvent Server sample ActiveMQ connectors for connecting to ActiveMQ message servers.
Apache License 2.0
6 stars 6 forks source link

support failover transport; update dependencies; add up-front validation #2

Closed brudo closed 7 years ago

brudo commented 8 years ago

Improved support for the failover:// transport in the ActiveMQ broker URI, and probably discovery:// as well, ensuring GeoEvent does not hang in the event that no brokers are available when the input is started, if the input is subsequently stopped (or if a defunct starter thread wakes up after it has been discarded).

The output transport has not yet been modified but could support similar changes.

Also updated the dependencies to use ActiveMQ 5.13.4, which is the latest major version at this time, and to remove unused optional libraries like Camel, hopefully improving quality while reducing overall JAR size.

Added version numbers to the JAR filename and removed support for older versions of GeoEvent Extension, but preserved compatibility with any existing configured inputs / connectors that were created for older builds of ActiveMQ for GeoEvent.

brudo commented 8 years ago

The last commit is not related to the failover support or new dependencies, but works around a bug in ActiveMQ through enhanced validation logic.

Incidentally, I noticed ActiveMQ 5.14.0 was released a few days ago, but I have not updated the dependencies to test if the bug mentioned is already resolved; I am still at 5.13.4.

Even if it is fixed in a future version of the ActiveMQ client library (which I hope it will be), I would still want to keep the up-front validation and detection of malformed properties in-place. A side benefit of this change is that the user is now notified of these errors while they are still editing an input in GeoEvent Manager, and cannot save or start that input before correcting such errors.

brudo commented 8 years ago

I don't think I am done with that validation fix / work-around quite yet as it turns out...

Although the Edit Input page remains open if validation failed and it does not appear to have saved, the changes are still persisted. So, if you cancel out after a validation error, the incorrect settings are still saved.

Also, you can start the input after it has been saved with invalid settings, in this manner. As such, the up-front validation only makes it difficult to accidentally configure a bad setting; it is not a substitute for run-time checking.

This also applies to the validation logic for Destination Type value. As a result, the code in setup defaults to Queue in some cases when it would have raised an error before, which could be seen as a regression.

When I address these remaining deficiencies (next few days), I'll just add the commits to the pull request here.

brudo commented 7 years ago

Although this has not been merged, I am closing and will park those changes in a branch on my fork.

brudo commented 7 years ago

I updated the dependencies to GeoEvent SDK 10.5.1 and ActiveMQ 5.15.0, and also removed dependencies that were previously added to enable experimental support for launching an embedded broker, and merged the changes back into my master. The version number exposed in the input / output definitions is left at 10.3.0 intentionally, for compatibility with previously defined connectors. Based on these updates I am reopening this pull request and you are welcome to reconsider merging it if you want.

hanoch commented 7 years ago

@brudo which version of GeoEvent Server did you test the transport with? did you test both inbound and outbound transports?

brudo commented 7 years ago

@hanoch we are running 10.4.1 in production and 10.5.1 in the lab. It works in both. Did not test outbound in latest round of updates; our use of ActiveMQ for GeoEvent is primarily inbound.

brudo commented 7 years ago

@hanoch I reviewed the changes and cleanup you made post-merge and it all looks good. I have merged back to my fork, except keeping the non-standard version numbers we're using locally.