Skip to content

Conversation

@tahmed42
Copy link

@tahmed42 tahmed42 commented Jun 3, 2014

Support using the import endpoint instead of the track endpoint when an event has a time that's older than 5 days in the past.

There's lots of ways to handle this but these changes were done in a manner to allow existing users to seamlessly continue using the package as they are, while allowing those that want the extra functionality to take advantage of it. This involves fitting the added logic into the existing code structure instead of rewriting a lot to make things ideal but then breaking the existing API in the process. So - some potential awkwardness for the sake of API compatibility.

Copy link
Author

Choose a reason for hiding this comment

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

The version will likely need to be manipulated in some way if this pull request goes anywhere.

@tahmed42
Copy link
Author

tahmed42 commented Jun 3, 2014

@tahmed42
Copy link
Author

tahmed42 commented Jun 3, 2014

Also note that no method was added for the import endpoint directly - it's only currently an option within track when the event is too old. This could certainly be added so long as its parameters were agreed upon (they'd likely mimic track's). They could both share common underlying code and just manipulate the endpoint name sent to the consumer's send method.

@tahmed42
Copy link
Author

tahmed42 commented Jun 3, 2014

It would be good to close this pull request and have a simple import method be defined and then rely on higher level calls to discern whether to track or import, leaving the individual methods single-purpose. Leaving this open for now to show what import functionality would take and how it could eventually work in tandem with track at some level, as well as to get clarifying questions answered such as the specifics of the timeframe past which the track endpoint is unusable.

@tahmed42 tahmed42 closed this Jun 3, 2014
@joeatwork
Copy link
Contributor

Thanks for submitting this one- we actually do have similar functions for the ruby lib, so I think support for the import endpoint definitely has a place in the library. We'll take a look at this PRQ (closed or open) and consider it when we push the feature!

So thanks! (and keep these coming! :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants