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

Add Timezone Support for ScheduleBuilder #138

Closed
wants to merge 4 commits into from
Closed

Conversation

vamsii777
Copy link

@vamsii777 vamsii777 commented Nov 20, 2024

There are two main ways to set the timezone for scheduling jobs:

  1. Using the in method
// Using full timezone identifier
app.queues.schedule(Cleanup()).in(timezone: "America/New_York").daily().(.midnight)

// Using timezone abbreviation
app.queues.schedule(Cleanup()).in(timezone: "EST").daily().(.midnight)
  1. Using timezone parameter in time methods
// Daily schedule with timezone
app.queues.schedule(Cleanup()).daily().at("15:30", timezone: "Europe/London")

// Using 24-hour format
app.queues.schedule(Cleanup()).daily().at(15, 30, timezone: "Asia/Tokyo")

// Using 12-hour format with AM/PM
app.queues.schedule(Cleanup()).daily().at(3, 30, .pm, timezone: "Australia/Sydney")

@grahamburgsma
Copy link

This seems like duplicated configuration as you can already achieve this using:

let calendar = Calendar.current
calendar.timeZone = TimeZone(identifier: "America/New_York")!

app.queues.schedule(Cleanup())
  .using(calendar)
  ...

- Full identifiers (e.g. "America/New_York")
- Abbreviations (e.g. "EST")
@vamsii777
Copy link
Author

This seems like duplicated configuration as you can already achieve this using:

let calendar = Calendar.current
calendar.timeZone = TimeZone(identifier: "America/New_York")!

app.queues.schedule(Cleanup())
  .using(calendar)
  ...

Thank you for the feedback @grahamburgsma! While .using(calendar) works, .in(timezone:) simplifies the API and improves developer experience by directly supporting timezone specification in the scheduling step.

API Consistency
The proposed .in(timezone:) modifier follows existing builder pattern and provides a more intuitive API that aligns with how developers think about scheduling across timezones.

app.queues.schedule(Cleanup())
.in(timezone: "America/New_York")
.daily()
.at("9:00am")

Developer Experience
While .using(calendar) works, it requires:

  • Create and configure a Calendar instance
  • Set the timezone on that Calendar

This approach avoids the need to configure a Calendar, making the code cleaner and more intuitive. Thanks again for sharing your thoughts!

@vamsii777
Copy link
Author

@gwynne Would you be able to review this PR at your earliest? If you believe it does not align with the current package, please let me know.

@vamsii777 vamsii777 closed this Nov 26, 2024
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