Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion slack_sdk/oauth/installation_store/internals.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from datetime import datetime
from datetime import datetime, timezone
from typing import Type, TypeVar, Union


Expand Down Expand Up @@ -28,6 +28,9 @@ def _timestamp_to_type(ts: Union[TimestampType, datetime, str], target_type: Typ
# see https://github.com/google/pytype/issues/1012

elif isinstance(ts, datetime):
if ts.tzinfo is None:
# naive datetime values (e.g., loaded from a database) are stored in UTC

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code is self explanatory 🙏 I don't think we need this comment

Once removed we should be good to merge 🚀

Suggested change
# naive datetime values (e.g., loaded from a database) are stored in UTC

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed in 9b07398, thanks!

ts = ts.replace(tzinfo=timezone.utc)
result = target_type(ts.timestamp())
elif isinstance(ts, str):
try:
Expand Down
13 changes: 13 additions & 0 deletions tests/slack_sdk/oauth/installation_store/test_internals.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import os
import sys
import time
import unittest
from datetime import datetime, timezone
from unittest import mock

import pytest

Expand Down Expand Up @@ -38,6 +41,16 @@ def test_timestamp_to_type(ts, target_type, expected_result):
assert result == expected_result


@pytest.mark.skipif(not hasattr(time, "tzset"), reason="time.tzset is not available on this platform")
def test_timestamp_to_type_naive_datetime_is_utc():
try:
with mock.patch.dict(os.environ, {"TZ": "America/New_York"}):
time.tzset()
assert _timestamp_to_type(datetime(2023, 11, 28, 22, 9, 7), int) == 1701209347
finally:
time.tzset()


def test_timestamp_to_type_invalid_str():
match = "Invalid isoformat string"
with pytest.raises(ValueError, match=match):
Expand Down
Loading