Skip to content

Conversation

notoraptor
Copy link
Contributor

Make sure GPUBilling and NodeGPUMapping objects use UTC datetimes (and not str datetimes anymore).

NB: We still cache slurm config files using local day, since we want to remember when (in local time) did we download these files.

@notoraptor notoraptor marked this pull request as ready for review October 23, 2025 16:23
Copy link
Collaborator

@abergeron abergeron left a comment

Choose a reason for hiding this comment

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

This looks ok, but I think that we should use UTC even in user-facing date parsing. Otherwise it might get confusing.

Also, this should absolutely get split into fetch and parse to avoid the awkwardness of "use cache if day is specified otherwise go fetch". It would also enable us to use the complete datetime for the cache timestamp because users wouldn't have to guess at the exact second the file was fetched to use the cache.

I'm ok with these things getting pushed to a second PR if you want to merge this first.

@notoraptor
Copy link
Contributor Author

notoraptor commented Oct 23, 2025

@abergeron OK ! I made a ticket about using complete datetime in UTC format to cache slurm files: https://mila-iqia.atlassian.net/browse/SARC-477

I will merge this PR and make a second PR later (this week or the next one).

@notoraptor notoraptor merged commit 1b0a1ed into master Oct 23, 2025
4 checks passed
@notoraptor notoraptor deleted the SARC-475-fix-date-in-gpumetrics branch October 23, 2025 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants