Skip to content
This repository has been archived by the owner on Jan 15, 2022. It is now read-only.

ability to specify minModificationTimeMillis as arg #107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

angadsingh
Copy link

as an argument to JobFilePreProcessor. Only files after this timestamp will be picked up. Still honor lastProcessRecord.

// Start of this time period is the end of the last period.
minModificationTimeMillis = lastProcessRecord
.getMaxModificationTimeMillis();
LOG.info("lastProcessRecord time: " + lastProcessRecord.getMaxModificationTimeMillis());
Copy link
Collaborator

Choose a reason for hiding this comment

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

A minor improvement suggestion -maybe have a variable that stores the lastProcessRecord.getMaxModificationTimeMillis() since it is being called thrice in this code block?

@vrushalic
Copy link
Collaborator

Overall, this pull request looks good to me (except for the minor improvement suggestion)

// Accept a minModificationTimeMillis. Don't process files before this time.
o = new Option("m", "minModificationTimeMillis", true,
"The minimum modification time of the file to be processed");
o.setArgName("minModificationTimeMillis");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is millis the most user-friendly, least error prone way to get this argument ?
Wouldn;t it be better to get something in a more huma-readable form ?
Perhaps yyyymmddhhMMss or something like that ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I actually think having a timestamp (long number) in epoch milliseconds is cleaner than a yyyymmdd etc format.

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Angad Singh seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants