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

[JENKINS-72653] fix timezone handling #301

Merged
merged 15 commits into from Mar 22, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Feb 4, 2024

JENKINS-72653 - fix timezone handling

the implementation to prefill the scheduled time with the global value was not working properly, when the timezone of the controller was different from the timezone that was configured. The implementation used java.util.Date which has only limited capabilities to work with timeszones.
This change replaces all date/time handling with the new classes from the java.time packages.
The timezone is no longer a free text field but a dropdown that gets filled with all available zone ids.
The datetime field to schedule a build is no longer locale dependent but uses a fixed format based on 24h format (see [JENKINS-72645]). It is optional to enter seconds.
The actual timezone that will be used is shown as field description.
The time input on global config allows to specify am/pm but also 24h formats, seconds are optional.

The config format has changed, but the old format can still be read. Once installed and config is saved, rolling back to a previous version requires changes to the config.

Before:
image
the format hint is wrong with german locale, it is unclear which timezone is used.

image
image
The default config is for 10 PM with Sydney time zone , but gets calculated to some strange value. The controller runs in UTC timezone but the time shown is what will be the time in Sydney when it is 10PM UTC (Sydney is UTC + 11).

After:
image
full format hint, timezone is shown

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No spotbugs warnings were introduced with my changes
  • I have interactively tested my changes

Types of changes

What types of changes does your code introduce?

  • Bug fix (non-breaking change which fixes an issue)

Further comments

If this is a relatively large or complex change, start the discussion by explaining why you chose the solution you did and what alternatives you considered.

the implementation to prefill the scheduled time with the global value
was not working properly, when the timezone of the controller was
different from the timezone that was configured. The implementation used
java.util.Date which has only limited capabilites to work with
timeszones.
This change replaces all date/time handling with new classes from the
java.time packages.
The timezone is no longer a free text field but a dropdown that gets
filled with all available zone ids.
The datetime field to schedule a build is no longer locale dependent but
uses a fixed format based on 24h format (see [JENKINS-72645]).
It is optional to enter seconds.
The actual timezone that will be used is shown as field description.
The time input on global config allows to specify am/pm but also 24h
formats, seconds are optional.
@mawinter69 mawinter69 requested a review from a team as a code owner February 4, 2024 00:16
@github-actions github-actions bot added documentation Improvements or additions to documentation tests Automated test addition or improvement labels Feb 4, 2024
@MarkEWaite MarkEWaite added bug Incorrect or flawed behavior and removed documentation Improvements or additions to documentation tests Automated test addition or improvement labels Feb 14, 2024
@github-actions github-actions bot added documentation Improvements or additions to documentation tests Automated test addition or improvement labels Feb 14, 2024
@MarkEWaite
Copy link
Contributor

Thanks very much! This is a very nice improvement! @darinpope had wanted us to make this type of improvement since shortly after we adopted the plugin.

Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks very much.

I noticed while doing data entry that the plugin previously accepted AM/PM suffix when entering a scheduled build time, but it no longer accepts the AM/PM suffix. Could you include support for the AM/PM suffix?

@mawinter69
Copy link
Contributor Author

Thanks very much.

I noticed while doing data entry that the plugin previously accepted AM/PM suffix when entering a scheduled build time, but it no longer accepts the AM/PM suffix. Could you include support for the AM/PM suffix?

done

previously it was possible to use 21:00:00 PM but as this is not really defined, this is no longer possible.

@MarkEWaite
Copy link
Contributor

This looks good in my testing.

My American brain does not immediately recognize ISO-8601 format dates (dd-MM-yyyy), but that is the international standard and it is unambiguous. I'm more accustomed to dates that include the first three letters of the month as an English language string (like 08-Mar-2024) , but that is not part of the ISO-8601 standard and is specific to English.

I'd like to do a little more testing before merging and releasing a new version. Thanks very much for the improvement!

@MarkEWaite MarkEWaite self-requested a review March 8, 2024 16:34
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Thanks. Sorry for the delay with the merge and release.

@MarkEWaite MarkEWaite merged commit a2b4f85 into jenkinsci:master Mar 22, 2024
17 checks passed
@baldpope
Copy link

baldpope commented Apr 8, 2024

I don't know if this is a me problem, but when scheduling a new job, the default date is tomorrow at 4am, which I'll assume is the value in GMT, even though the UI implies the timezone is America/Chicago. Shouldn't the default date/time (tonight at 10pm) be displayed correctly in the UI, even if it's stored in GMT?

The default value:
image

For reference, I'm in America/Chicago TZ, and attempting to schedule 'now' (951am on 4/8/2024) for 930pm tonight

@mawinter69
Copy link
Contributor Author

The value you enter will be applied to the mentioned timezone (America/Chicago). The timezone and the default start time in that timezone are configured in the system configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect or flawed behavior documentation Improvements or additions to documentation tests Automated test addition or improvement
Projects
None yet
3 participants