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

OTP-1465 Schedule Trip History Uploade #272

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

JymDyerIBI
Copy link
Contributor

@JymDyerIBI JymDyerIBI commented Nov 22, 2024

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JavaDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

OTP-1465: Schedule initial trip history upload at 3:00 AM instead of right away. Utility method finds next occurrence of 3:00 AM (or any time of day), which will be today or tomorrow, and scheduler is invoked with an initial delay to that time.

A new CONNECTED_DATA_PLATFORM_TRIP_HISTORY_UPLOAD_START_TIME configuration setting is introduced with a default value of "03:00".

Some OTP-1444 code was written concurrently, a different upload/transfer job. This same code could be used for that, but the two branches would need to be merged.

@JymDyerIBI JymDyerIBI self-assigned this Nov 22, 2024
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Sorry, I should have mentioned that hourly uploads are not subject to the 3am start.

var now = DateTimeUtils.nowAsZonedDateTime(DateTimeUtils.getOtpZoneId());
var timeOfDay = LocalTime.parse(CONNECTED_DATA_PLATFORM_TRIP_HISTORY_UPLOAD_START_TIME);
var startAt = DateTimeUtils.getNextTimeFrom(timeOfDay, now);
long initialDelayMinutes = Duration.between(now, startAt).toMinutes();
Copy link
Collaborator

Choose a reason for hiding this comment

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

One detail that I overlooked is that the configured start time should only apply if the uploads are daily (CONNECTED_DATA_PLATFORM_REPORTING_INTERVAL = "daily"). If the reporting interval is hourly, the job should still start immediately.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Extract a method to compute the initial delay (lines 122 to 125), and place it in DateTimeUtils or Scheduler, and add a unit test for that method.

var zdt = ZonedDateTime.of(2024, 11, 11, 15, 34, 17, 0, DateTimeUtils.getOtpZoneId());
var timeOfDay = LocalTime.parse("05:15");
var zdtNext = DateTimeUtils.getNextTimeFrom(timeOfDay, zdt);
assertTrue(zdtNext.isAfter(zdt));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you are using an absolute date zdt, can you assert on the date, hour, and minutes instead?

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