Skip to content

Support setting and displaying timezone with the core metrics plugin #19527

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

Merged
merged 2 commits into from
May 7, 2025

Conversation

natefoo
Copy link
Member

@natefoo natefoo commented Feb 3, 2025

I confused myself and spent too much time this morning looking at the zoneless time displayed under the core metrics for a job, hence this effort to clarify.

When the timezone metric param is unset the behavior remains the same as before. I wanted to get the local server timezone so that we could at least display the name/offset in this case, but doing so is non-trivial using the standard lib. We do have both dateutil (direct) and tzlocal (indirect) as dependencies, but only use dateutil in one place and I didn't want to add another use on a library we could easily rewrite out.

Before / no timezone set image
Set to UTC image
Set to EST5EDT image

For a global server like usegalaxy.org it makes sense to just set it to UTC imo.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@natefoo
Copy link
Member Author

natefoo commented Feb 3, 2025

Hrm, I imagined that we had dropped 3.8, but I guess not yet?

@nsoranzo
Copy link
Member

nsoranzo commented Feb 4, 2025

Hrm, I imagined that we had dropped 3.8, but I guess not yet?

I think Galaxy 25.0 is a good time to drop Python 3.8, happy to discuss at the Backed WG meeting.

@natefoo natefoo marked this pull request as draft February 4, 2025 16:25
@natefoo
Copy link
Member Author

natefoo commented Feb 4, 2025

Converted to draft until 3.8 is dropped.

@ahmedhamidawan ahmedhamidawan requested a review from jdavcs May 6, 2025 15:44
@natefoo natefoo force-pushed the core-metrics-tz branch from fe8c2ed to decf037 Compare May 6, 2025 15:46
@natefoo natefoo marked this pull request as ready for review May 6, 2025 15:46
@jdavcs
Copy link
Member

jdavcs commented May 7, 2025

The 4 remaining linting errors are not related to this PR. Merging this, will fix or open issue for linting errors.

@jdavcs jdavcs merged commit 5a1b0ba into galaxyproject:dev May 7, 2025
43 of 52 checks passed
Copy link

github-actions bot commented May 7, 2025

This PR was merged without a "kind/" label, please correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants