Skip to content

fix: move away from datetime.utcfromtimestamp for the state and installation stores - #1798

Merged
WilliamBergamin merged 5 commits into
mainfrom
move-away-from-datetime.utcfromtimestamp
Dec 5, 2025
Merged

fix: move away from datetime.utcfromtimestamp for the state and installation stores#1798
WilliamBergamin merged 5 commits into
mainfrom
move-away-from-datetime.utcfromtimestamp

Conversation

@WilliamBergamin

Copy link
Copy Markdown
Contributor

Summary

This PR aims to resolve#1676 by favoring the following

  • datetime.utcfromtimestamp(...) -> datetime.fromtimestamp(..., tz=timezone.utc)
  • datetime.utcnow() -> datetime.now(tz=timezone.utc)

Some investigation was required to ensure this change would not impact existing application. Since the timestamps are stored and compared as integers this change should not cause any breaking change.

Testing

Unit test coverage should be sufficient

Category

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.signature (Request Signature Verifier)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.models (UI component builders)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.rtm_v2 (RTM client)
  • /docs (Documents)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

@WilliamBergaminWilliamBergamin added this to the 3.40.0 milestone Dec 1, 2025
@WilliamBergaminWilliamBergamin self-assigned this Dec 1, 2025
@WilliamBergamin
WilliamBergamin requested a review from a team as a code ownerDecember 1, 2025 17:18
@WilliamBergaminWilliamBergamin added bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented semver:patch Version: 3x oauth area:async python Pull requests that update Python code dependencies Pull requests that update a dependency file labels Dec 1, 2025
@codecov

codecovBot commented Dec 1, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.91%. Comparing base (dfa5e5f) to head (c3abf8f).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@ Coverage Diff @@## main #1798 +/- ##
=======================================
Coverage 83.90% 83.91% =======================================
Files 115 115 Lines 13080 13080 =======================================
+ Hits 10975 10976 +1 + Misses 2105 2104 -1 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@WilliamBergaminWilliamBergamin changed the title Move away from datetime.utcfromtimestampfix: move away from datetime.utcfromtimestampDec 1, 2025

@zimegzimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Super nice changes! I left a comment around the effects of these changes and I'm also curious if #1731 might be addressed with this too?

I'm marking this as "approved" but do let me know if more review and testing can be helpful 🙏 ✨

Comment on lines -106 to +108
"installed_at": datetime.utcfromtimestamp(self.installed_at),
"installed_at": datetime.fromtimestamp(self.installed_at, tz=timezone.utc),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤩 praise: @WilliamBergamin I'm a huge fan of keeping all representations of time in the UTC timezone!

👁️‍🗨️ question: Would adjacent implementations also require a similar change? I find this initial value might be generated with an offset:

ifinstalled_atisNone:
self.installed_at=datetime.now().timestamp()
else:
self.installed_at=_timestamp_to_type(installed_at, float)

📚 https://docs.python.org/3/library/datetime.html#datetime.datetime.now

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Its better if we open up another PR for these changes and resolve #1731

@zimegzimeg changed the title fix: move away from datetime.utcfromtimestampfix: move away from datetime.utcfromtimestamp for the state and installation storesDec 3, 2025
@WilliamBergamin
WilliamBergamin merged commit b7eb2f9 into mainDec 5, 2025
16 checks passed
@WilliamBergamin
WilliamBergamin deleted the move-away-from-datetime.utcfromtimestamp branch December 5, 2025 20:55
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:asyncbugM-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documenteddependenciesPull requests that update a dependency fileoauthpythonPull requests that update Python codesemver:patchVersion: 3x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: datetime.utcfromtimestamp is deprecated and scheduled for removal

2 participants

@WilliamBergamin@zimeg