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

[Bug] Integration test gives an error for the JST timezone #7664

Closed
2 tasks done
Tracked by #7808
d-kaneshiro opened this issue May 19, 2023 · 3 comments · Fixed by #7665
Closed
2 tasks done
Tracked by #7808

[Bug] Integration test gives an error for the JST timezone #7664

d-kaneshiro opened this issue May 19, 2023 · 3 comments · Fixed by #7665
Labels
bug Something isn't working flaky_test

Comments

@d-kaneshiro
Copy link
Contributor

d-kaneshiro commented May 19, 2023

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

Error at dbt-core/tests/functional/simple_snapshot/test_hard_delete_snapshot.py:97 when running make integration.

Expected Behavior

Integration tests complete successfully.

Steps To Reproduce

I forked the repository and built the necessary environment according to “CONTRIBUTING.md”.
Below is the command I executed.

  1. Build venv environment
  • python3 -m venv env
  • source env/bin/activate
  • pip install --upgrade pip
  1. Build dbt environment
  • make dev
  1. Build DB environment
  • make setup-db
  1. Run unit tests
  • make test
  1. Run integration tests
  • make integration

In addition, in “~/.dbt/profiles.yml”,

default:
   target: dev
   outputs:
     devs:
       type: postgres
       host: localhost
       user: root
       password: password
       port: 5432
       dbname: postgres
       schema: public
       threads: 4

Relevant log output

>           assert result[-1].replace(tzinfo=currenttz()) >= invalidated_snapshot_datetime
E           AssertionError: assert datetime.datetime(2023, 5, 15, 7, 36, 0, 679815, tzinfo=datetime.timezone(datetime.timedelta(seconds=32400), 'JST')) >= datetime.datetime(2023, 5, 15, 7, 36, 0, 58485, tzinfo=<UTC>)
E            +  where datetime.datetime(2023, 5, 15, 7, 36, 0, 679815, tzinfo=datetime.timezone(datetime.timedelta(seconds=32400), 'JST')) = <built-in method replace of datetime.datetime object at 0x11a20f2a0>(tzinfo=datetime.timezone(datetime.timedelta(seconds=32400), 'JST'))
E            +    where <built-in method replace of datetime.datetime object at 0x11a20f2a0> = datetime.datetime(2023, 5, 15, 7, 36, 0, 679815).replace
E            +    and   datetime.timezone(datetime.timedelta(seconds=32400), 'JST') = currenttz()

Environment

- OS:macOS Catalina 10.15.7
- Python:3.10.5
- dbt:1.6.0-b1

Which database adapter are you using with dbt?

postgres

Additional Context

No response

@d-kaneshiro d-kaneshiro added bug Something isn't working triage labels May 19, 2023
@d-kaneshiro d-kaneshiro mentioned this issue May 19, 2023
6 tasks
@dbeatty10 dbeatty10 changed the title [Bug]For JST the integration test gives an error [Bug] For JST the integration test gives an error May 19, 2023
@dbeatty10 dbeatty10 changed the title [Bug] For JST the integration test gives an error [Bug] Integration test gives an error for the JST timezone May 19, 2023
@dbeatty10
Copy link
Contributor

Thanks for reporting this issue and opening up a pull request @d-kaneshiro !

We'd like to reproduce this error before reviewing the pull request. I suspect it has something to do with the test working for an IANA timezone behind UTC (like America/Denver) rather than ahead (like Asia/Tokyo).

Could you check if there any timezone setting on your local computer that will allow this test to pass? i.e., if you set your system time to something behind UTC (like America/Denver), does the integration test pass?

@d-kaneshiro
Copy link
Contributor Author

d-kaneshiro commented May 22, 2023

Thanks for commenting @dbeatty10.
The integration test passed when I inserted the below code in the target file(dbt-core/tests/functional/simple_snapshot/test_hard_delete_snapshot.py).

os.environ['TZ'] = 'America/Denver'
time.tzset()

On the other hand, in the case of JST, the problem has not been resolved.
Wouldn't it be acceptable to unify to UTC as shown in this PullRequests?

@dbeatty10
Copy link
Contributor

Thanks for looking into that @d-kaneshiro ! 🏅

I just wanted to gain an understanding of what was causing the issue. Your experiment gives some evidence that this integration test isn't working for timezones with a positive UTC offset, but does work for those with a negative UTC offset.

Unifying to UTC in your pull request might be the solution -- I'll comment further within your pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working flaky_test
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants