Skip to content

Conversation

@WilliamBergamin
Copy link
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.

@WilliamBergamin WilliamBergamin added this to the 3.40.0 milestone Dec 1, 2025
@WilliamBergamin WilliamBergamin self-assigned this Dec 1, 2025
@WilliamBergamin WilliamBergamin requested a review from a team as a code owner December 1, 2025 17:18
@WilliamBergamin WilliamBergamin 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
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.91%. Comparing base (4601f8a) to head (d7caaf4).
✅ 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.

@WilliamBergamin WilliamBergamin changed the title Move away from datetime.utcfromtimestamp fix: move away from datetime.utcfromtimestamp Dec 1, 2025
Copy link
Member

@zimeg zimeg left a comment

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
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:

if installed_at is None:
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

@zimeg zimeg changed the title fix: move away from datetime.utcfromtimestamp fix: move away from datetime.utcfromtimestamp for the state and installation stores Dec 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:async bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented dependencies Pull requests that update a dependency file oauth python Pull requests that update Python code semver:patch Version: 3x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: datetime.utcfromtimestamp is deprecated and scheduled for removal

3 participants