-
Notifications
You must be signed in to change notification settings - Fork 1
Faker provides suitable time/date generation #15
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
base: main
Are you sure you want to change the base?
Conversation
Remove use of `now_z` macro, since Faker can provide the same (or sufficiently-similar) functionality. Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the custom now_z macro in favor of using Faker's built-in date/time generation capabilities for mock data.
- Replaces the custom
now_zlambda function with Faker'sdate_time_this_monthmethod - Introduces
tz_utcas a global variable to support timezone-aware datetime generation - Maintains the ISO 8601 format with "Z" suffix for UTC timestamps
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/lfx_v2_mockdata/__init__.py |
Removed the now_z lambda function from Jinja2 globals and added tz_utc (datetime.UTC) to support timezone-aware Faker date generation |
playbooks/projects/recreate_root_project/root.yaml |
Updated the created_at timestamp generation to use fake.date_time_this_month() with timezone parameter instead of the removed now_z() macro |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parent_uid: "" | ||
| stage: "" | ||
| {% set created_at = now_z() %} | ||
| {% set created_at = fake.date_time_this_month(False, False, tz_utc).isoformat().replace("+00:00", "Z") %} |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new fake.date_time_this_month() call includes a timezone parameter tz_utc, but other Faker date_time methods throughout the codebase (e.g., in board_meeting.yaml) do not pass a timezone parameter. This inconsistency could lead to timezone-related issues. Consider either: (1) adding timezone parameters to other Faker date_time calls, or (2) verifying that Faker's default behavior produces UTC timestamps that don't require explicit timezone handling.
| {% set created_at = fake.date_time_this_month(False, False, tz_utc).isoformat().replace("+00:00", "Z") %} | |
| {% set created_at = fake.date_time_this_month(False, False).isoformat().replace("+00:00", "Z") %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The others are technically zoneless objects for the current environment's time, which are treated as UTC without any actual conversion. So they do work OK for fake times, but there is a slight risk that past times might actually be later the same day, and vice versa. In #12 I've suggested that we add TZ=UTC to the setup-env so that the zoneless ones work better.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
Remove use of
now_zmacro, since Faker can provide the same (or sufficiently-similar) functionality.