nuodb-python icon indicating copy to clipboard operation
nuodb-python copied to clipboard

Support timezone-aware datetime objects

Open butson opened this issue 8 months ago • 5 comments

connection:

  • Added support for explicit TimeZone connection property

datatype:

  • Use Julian calendar for date/datetime before 1582 Oct 4. When the Gregorian calendar was introduced: . The day after October 4, 1582 (Julian) became October 15, 1582 (Gregorian). . That 10-day jump is not accounted for in C's calendar logic. . Julian calendar handles leap years differently. The utc calendar the engine is using understands this the python functions based on C calendar logic do not.
  • time.mktime , time.localtime don't support older dates while engine does changing to use calendar above and calculate time, avoid these issues.
  • use of datetime + timezone should handle daylight savings times and not be off by an hour.

tests:

  • Use connection property TimeZone to change timezone
  • Support timezone-aware datetime
  • Use pytz correctly with localize if pytz is used
  • Use zoneinfo.ZoneInfo if available

encodedsession:

  • Manage and verify TimeZone settings on per connection basis
  • Allows connections with different TimeZone setting from same application

requirements.txt:

  • Add requirement for tzlocal, should be available on all supported python versions. There is a change in api between releases , this is handled in the code.

butson avatar Apr 25 '25 16:04 butson

I expect the cursor test failure is related to the callback to engine to get the real time zone used.

butson avatar Apr 28 '25 16:04 butson

Hi @butson I will try to get to the review proper this week. Just to start out, there are a number of PEP8 violations in the new code.

What I recommend for finding them, if you don't already have an IDE which is capable of enabling flake8 or similar add-ons to do this sort of syntax checking, is to get Visual Studio Code (this is not the full Visual Studio suite, it's a stand-alone, open source editor which runs on all major plaforms: Windows, GNU/Linux, and MacOS): https://code.visualstudio.com/ . Then you should configure it for Python development by installing these extensions: Python (Microsoft), Pylance (Microsoft), and Flake8.

Once you have done that (I recommend restarting after you install the extensions) you can visit the files in your Git repository in the editor, and use CTRL-SHIFT-M to open the "Problems" window and see all the issues reported there. If there are errors not in code you didn't write/update you don't need to worry about it. In particular, the "imported but not used" messages should mostly be ignored, because these tools don't grok the "type:" comments we're using with mypy. But, there are lots of whitespace issues such as, missing spaces after commas etc. in the changed code: these need to be addressed.

madscientist avatar May 12 '25 21:05 madscientist

Secondly, the error found by circleci in the test_result_set_gets_closed test is due to an incorrect implementation of the new method _set_timezone(). If you write it like this, instead, it will work without errors:

          stmt = self.create_statement()
          self.execute_statement(stmt, "select value from system.connectionproperties where property='TimeZone'")
          rset = self.fetch_result_set(stmt).fetchone()
          self.timezone_name = rset[0]
          self.close_statement(stmt)

Note, you can run these tests locally on your system; you need to set it up but once you do, you can verify changes before you push to GitHub and wait for the CircleCi results. If you're not sure how to do it let me know.

madscientist avatar May 12 '25 21:05 madscientist

Thanks @butson . Note, there are a lot of mypy and pylint errors as well. You can run make mypy and make pylint to run them. You'll have to install them first (I don't know exactly how to do that on MacOS; I only use Linux myself... let me know if you can't figure it out).

On master, mypy is 100% clean and pylint only complains about a few TODO comments.

madscientist avatar May 15 '25 19:05 madscientist

Paul - I made the changes recommend here. I've updated the branch to just one commit, added test cases, and changed calendar to use the package jdcal. I've run the test on:

Python 2.7.18, 3.8.20, 3.12.8

checked the code with mypy and flake8. I've not been able to run pylint without a bunch of output from code that is not necessarily what I've updated. So any instructions for configuring and running pylint might help if there are issues there.

butson avatar May 29 '25 16:05 butson

Paul, test pass now. and ready for your review.

butson avatar May 30 '25 22:05 butson