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.

Sign up for GitHub

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

Jump to bottom

jobs: change SHOW JOBS timestamps to timestamptz #108353

Merged
merged 2 commits into from
Aug 9, 2023
Merged

jobs: change SHOW JOBS timestamps to timestamptz #108353

merged 2 commits into from
Aug 9, 2023

Conversation

dt
Copy link
Member

@dt dt commented Aug 8, 2023

Release note (sql change): SHOW JOBS now returns times (created, last_run, etc) using the TimestampTZ column type instead of the Timestamp type, meaning they are now rendered using the session offset.

Epic: CRDB-24406.

@dt dt requested a review from adityamaru August 8, 2023 13:27
@dt dt requested review from a team as code owners August 8, 2023 13:27
@cockroach-teamcity
Copy link
Member

This change is Reviewable

adityamaru
adityamaru approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@adityamaru adityamaru left a comment

Choose a reason for hiding this comment

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

LGTM, do we call this out as a breaking change? Also if we can run ./dev build and make sure the jobs page is fine with this change?

@dt
Copy link
Member Author

dt commented Aug 8, 2023

do we call this out as a breaking change?

Yesterday @rafiss said these do not need to be labeled as breaking changes:
#108290 (review)

@dt
server: support DTimestampTZ types in status server scans
8769bec 
Release note: none.
Epic: CRDB-24406.
@dt dt requested a review from a team August 8, 2023 19:00
@dt dt requested a review from a team as a code owner August 8, 2023 19:00
@dt dt requested review from abarganier and removed request for a team August 8, 2023 19:00
@dt
Copy link
Member Author

dt commented Aug 8, 2023 •
edited
Loading

run ./dev build and make sure the jobs page is fine

This was a good shout. The jobs page was not fine, since some code in server/admin.go was only handing *DTimestamp. I've added a commit and ensured it is now fine.

@dt
Copy link
Member Author

dt commented Aug 9, 2023

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 9, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 9, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 9, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 9, 2023

Build failed (retrying...):

@yuzefovich
Copy link
Member

There appear to be some legitimate failures on this PR.

bors r-

@craig
Copy link
Contributor

craig bot commented Aug 9, 2023

Canceled.

@dt
jobs: change SHOW JOBS timestamps to timestamptz
c0dc380 
Release note (sql change): SHOW JOBS now returns times (created, last_run, etc) using
the TimestampTZ column type instead of the Timestamp type, meaning they are now
rendered using the session offset.

Epic: CRDB-24406.
@dt
Copy link
Member Author

dt commented Aug 9, 2023

some legitimate failures on this PR.

Whoops, sorry @yuzefovich! I'd scanned the red PR CIs and saw two failures (ttl job, then sql/copy) which I knew were unrelated, but missed the logic test that needed to be updated. Thanks the heads up.

fixed now.

bors r+

@craig
Copy link
Contributor

craig bot commented Aug 9, 2023

Build succeeded:

@craig craig bot merged commit 66c9e4f into cockroachdb:master Aug 9, 2023
@cockroach-teamcity cockroach-teamcity mentioned this pull request Aug 10, 2023
Closed
@dt dt deleted the jobs-tz branch August 23, 2023 13:54
@dt dt mentioned this pull request Sep 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adityamaru adityamaru adityamaru approved these changes

@ecwall ecwall ecwall approved these changes

@abarganier abarganier Awaiting requested review from abarganier

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dt @cockroach-teamcity @yuzefovich @ecwall @adityamaru