Skip to content

Add support for event ID generation#1

Open
esammer wants to merge 2 commits intomasterfrom
feature/event-id-generation
Open

Add support for event ID generation#1
esammer wants to merge 2 commits intomasterfrom
feature/event-id-generation

Conversation

@esammer
Copy link
Member

@esammer esammer commented Aug 28, 2016

No description provided.

- Includes the EventIdGenerator interface and implementations in the new package
  com.osso.event.id.
- Javadoc exists for all classes.
- Contains the addition of Guava 15. It may make sense to shade this in a follow up commit.
- These docs were lost in a bad `git reset` and should be have included in the
  previous commit.
<!-- Project compile dependencies. -->

<dependency>
<groupId>com.google.guava</groupId>
Copy link
Member Author

Choose a reason for hiding this comment

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

We're probably going to want to shade this to prevent issues for users.

@joey
Copy link
Member

joey commented Aug 30, 2016

I'd rather implement ID generation using the builder pattern rather than including the static populateId() method. The only use case for the static method is for modifying the ID in place. I could maybe see that as useful if you're upgrading to a new standard and want to apply a new ID generation technique to data sent over the wire using the older standard. Then again, you could have the builder take in an existing Event to seed the fields at the cost of allocating a new Event object.

@esammer
Copy link
Member Author

esammer commented Aug 30, 2016

Thanks for the feedback. The only challenge I see with the custom builder is that we'd have to supply our own templates for Avro code gen so we could sneak it in to the generated class. Does that sound awful or like a reasonable solution? Or, is my OCD working overtime and it's fine for the builder to live outside of the Event class (which, admittedly, I don't like)?

@joey
Copy link
Member

joey commented Aug 30, 2016

I'd probably have the builder live outside the Event class, but maybe that's because I'm nervous when messing with code gen stuff. Does Avro code gen support giving them templates? If so, I'd be down to use that.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments