Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Add the RandomDiscreteTimestampGenerator to generate a range of #1004

Closed
wants to merge 1 commit into from

Conversation

manolama
Copy link
Collaborator

@manolama manolama commented Aug 6, 2017

timestamps in a non-repeating random order.
Modify UnixEpochTimestampGenerator so that the random generator can
extend it.

@manolama
Copy link
Collaborator Author

manolama commented Aug 8, 2017

@risdenk @busbey Mind taking a quick look at these please? Last few commits I need before the time series workload can be PR'd. Thanks!

@risdenk risdenk self-requested a review August 10, 2017 02:13
@risdenk risdenk added this to the 0.13.0 milestone Aug 10, 2017
Copy link
Collaborator

@risdenk risdenk left a comment

Choose a reason for hiding this comment

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

One minor nit but otherwise looks good.

break;
case DAYS:
currentTimestamp = (System.currentTimeMillis() / 1000) +
getOffset(intervalOffset);
startTimestamp = currentTimestamp;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: could do this startTimestamp = currentTimestamp could go right before the return after the switch/case

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, cleaned it up. Thanks!

timestamps in a non-repeating random order.
Modify UnixEpochTimestampGenerator so that the random generator can
extend it.
@manolama
Copy link
Collaborator Author

Merged in b945c66, thanks!

@manolama manolama closed this Aug 10, 2017
@manolama manolama mentioned this pull request Sep 26, 2017
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.

None yet

2 participants