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

feat!: do not get TZ info from server local env #2905

Merged
merged 4 commits into from
Dec 12, 2023

Conversation

waynexia
Copy link
Member

@waynexia waynexia commented Dec 11, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Remove the logic of fetching server's local timezone as the default timezone. Use Zulu (UTC+0) by default. We can provide a method to set per-session timezone in the future, as requested in #2893

Warning

This is a breaking change. After this patch, the server-side timezone will default to GMT instead of environment $TZ. It would be configurable in the future (#2907)

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

Closes #2856
Part of #2907

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Copy link

codecov bot commented Dec 11, 2023

Codecov Report

Merging #2905 (4a66b7b) into develop (58183fe) will decrease coverage by 0.50%.
Report is 8 commits behind head on develop.
The diff coverage is 100.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2905      +/-   ##
===========================================
- Coverage    85.01%   84.52%   -0.50%     
===========================================
  Files          753      756       +3     
  Lines       118592   119453     +861     
===========================================
+ Hits        100820   100966     +146     
- Misses       17772    18487     +715     

@killme2008
Copy link
Contributor

@waynexia Is this a breaking change?

@waynexia waynexia changed the title feat: do not get TZ info from server local env feat!: do not get TZ info from server local env Dec 12, 2023
@github-actions github-actions bot added the breaking-change This pull request contains breaking changes. label Dec 12, 2023
@waynexia
Copy link
Member Author

@waynexia Is this a breaking change?

Yes, I've changed the title

@MichaelScofield
Copy link
Collaborator

A user says he's using Influxdb line protocol with explicit timestamp to write, and read with timestamp (instead of the date time like "2023-12-12 17:45:00"). So I think this breaking change is fine.

@killme2008
Copy link
Contributor

Let's move on!

@killme2008 killme2008 added this pull request to the merge queue Dec 12, 2023
Merged via the queue into GreptimeTeam:develop with commit 0ce2b50 Dec 12, 2023
19 checks passed
@waynexia waynexia deleted the ts-without-tz branch December 13, 2023 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This pull request contains breaking changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't select data by timestamp equals a literal
3 participants