search

keenlabs / KeenClient-Java

Official Java client for the Keen IO API. Build analytics features directly into your Java apps.
https://keen.io/docs
MIT License
74 stars 43 forks source link

KeenCallback with event map parameters #32

Closed mikereinhold closed 7 years ago

mikereinhold commented 9 years ago

I have a use case for the KeenCallback where I would want to retry sending an event, persist the event for later, or log information that would help determine which event failed to publish.

Unfortunately, the current KeenCallback interface does not allow for Keen to pass the event data (project, collection, event properties, keen properties) to the callback so that further action can be taken (such as retrying, detailed logging, event storage, etc).

Adding these parameters to the callback methods or creating additional callback methods would constitute a breaking change to the interface as all existing implementations would need to be modified in order to compile with the new version of the interface.

My suggestion would be to create an additional callback interface, perhaps KeenEventCallback or something similar, that extends the KeenCallback interface with these new methods. Developers that need this type of callback could implement the advanced interface and pass it to the KeenClient during event publishing.

The KeenClient would need to be modified to check if the callback is an advanced callback and, if so, call the appropriate new callback method in place of or in addition to the original callback method.

If you think that other users of the Keen Java SDK would benefit from this feature, I would be happy to discuss further and put together a PR.

joshed-io commented 9 years ago

I think this is a great idea. Not having enough information in the callback sounds like a real pain.

At first blush I think we could just do a breaking change here. Having one type of callback will be cleaner in the future. Users upgrading will see an error at compile-time and we can instruct them on how to fix it.

Open to thoughts on any of that, but very willing to entertain and ship out a PR for this. Thanks @mikereinhold!

mikereinhold commented 9 years ago

I agree that a single callback interface is simpler in the long run. I'm usually not a fan of breaking changes, but as long as the SDK version increments properly to indicate that an upgrade to this version will require code change.

What makes mes comfortable with the breaking change is that the code change itself is simple enough - just a change to the method signature. Any callbacks that don't require the additional information can simply disregard the parameters.

Geeber commented 9 years ago

We're in the process of talking about a 3.0 release with some other breaking changes, so maybe we could wrap this in with that work? I think for now it would be good to use the interface extension approach outlined, and we can ship that as part of a 2.1 or 2.2 release.

mikereinhold commented 9 years ago

That makes sense to me - use the interface extension for the duration of the 2.x version and then implement the new callback signature in the 3.0 release. I'll start with a PR for the interface extension and will be happy to help however necessary.

On Mon, Dec 8, 2014 at 4:39 PM, Kevin Litwack notifications@github.com wrote:

We're in the process of talking about a 3.0 release with some other breaking changes, so maybe we could wrap this in with that work? I think for now it would be good to use the interface extension approach outlined, and we can ship that as part of a 2.1 or 2.2 release.

— Reply to this email directly or view it on GitHub https://github.com/keenlabs/KeenClient-Java/issues/32#issuecomment-66192323 .

Geeber commented 9 years ago

Awesome, thanks!

josephwegner commented 7 years ago

Looks like this was resolved. Closing.