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

SNOW-1776252: Prefer stdlib tzinfo objects over pytz's #2098

Open
Kache opened this issue Oct 30, 2024 · 7 comments
Open

SNOW-1776252: Prefer stdlib tzinfo objects over pytz's #2098

Kache opened this issue Oct 30, 2024 · 7 comments
Assignees
Labels
feature status-triage_done Initial triage done, will be further handled by the driver team

Comments

@Kache
Copy link
Contributor

Kache commented Oct 30, 2024

What is the current behavior?

Uses pytz's tzinfo objects

The following queries:

SELECT TO_TIMESTAMP_TZ('2000-01-01 11:11:11+00:00')
SELECT TO_TIMESTAMP_TZ('2000-01-01 11:11:11+01:00')

Are returned by snowflake-connector and printed out (respectively) as:

datetime.datetime(2000, 1, 1, 11, 11, 11, tzinfo=<UTC>)
datetime.datetime(2000, 1, 1, 11, 11, 11, tzinfo=pytz.FixedOffset(60))

What is the desired behavior?

Use stdlib tzinfo objects

For example, if _generate_tzinfo_from_tzoffset() was changed like:

  def _generate_tzinfo_from_tzoffset(tzoffset_minutes: int) -> tzinfo:
      """Generates tzinfo object from tzoffset."""
-     return pytz.FixedOffset(tzoffset_minutes)
+     return dt.timezone(dt.timedelta(minutes=tzoffset_minutes))

then the previous examples would instead print out:

datetime.datetime(2000, 1, 1, 11, 11, 11, tzinfo=datetime.timezone.utc)
datetime.datetime(2000, 1, 1, 11, 11, 11, tzinfo=datetime.timezone(datetime.timedelta(seconds=3600)))

How would this improve snowflake-connector-python?

  • the repr() of returned dt.datetime objects is valid Python (the <UTC> is not)
  • removes a dependency in favor of stdlib

References and other background

worth considering: https://blog.ganssle.io/articles/2018/03/pytz-fastest-footgun.html

@github-actions github-actions bot changed the title Prefer stdlib tzinfo objects over pytz's SNOW-1776252: Prefer stdlib tzinfo objects over pytz's Oct 30, 2024
@sfc-gh-sghosh sfc-gh-sghosh self-assigned this Nov 12, 2024
@sfc-gh-sghosh
Copy link

Hello @Kache ,

Thanks for raising the request; we will review it internally and get back to you.

Regards,
Sujan

@sfc-gh-sghosh sfc-gh-sghosh added status-triage Issue is under initial triage and removed needs triage labels Nov 12, 2024
@sfc-gh-sghosh
Copy link

Hello @Kache ,

The team is working and looking into it, there is no ETA as of now.

Regards,
Sujan

@sfc-gh-sghosh sfc-gh-sghosh added status-triage_done Initial triage done, will be further handled by the driver team and removed status-triage Issue is under initial triage labels Nov 18, 2024
@sfc-gh-sghosh
Copy link

Hello @Kache ,

Do you have an account in Snowflake? could you let us know?

Regards,
Sujan

1 similar comment
@sfc-gh-sghosh
Copy link

Hello @Kache ,

Do you have an account in Snowflake? could you let us know?

Regards,
Sujan

@Kache
Copy link
Contributor Author

Kache commented Dec 12, 2024

I'm using my employer's account

@sfc-gh-sghosh
Copy link

Hi kache,

Thanks for the update, whats the name of your employer? do they have aan ccount in Snowflake?
You can raise an official support ticket to share above details.
https://community.snowflake.com/s/article/How-To-Submit-a-Support-Case-in-Snowflake-Lodge

@Kache
Copy link
Contributor Author

Kache commented Dec 20, 2024

Sure, I'll do that

update I've opened support case 00918876

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

3 participants