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

Tests are failing in CI with an ISO format error #3717

Closed
wenzeslaus opened this issue May 18, 2024 · 6 comments · Fixed by #3721
Closed

Tests are failing in CI with an ISO format error #3717

wenzeslaus opened this issue May 18, 2024 · 6 comments · Fixed by #3721
Labels
bug Something isn't working CI Continuous integration
Milestone

Comments

@wenzeslaus
Copy link
Member

The tests are failing in CI with this weird error but the build passes locally. Also, the string that is said to be invalid is actually a valid isoformat string.

gnu/docs/html/g.gui.timeline.html
Traceback (most recent call last):
  File "/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/utils/mkhtml.py", line 921, in <module>
    git_commit = get_last_git_commit(
  File "/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/utils/mkhtml.py", line 397, in get_last_git_commit
    return parse_git_commit(
  File "/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/utils/mkhtml.py", line 242, in parse_git_commit
    git_log["date"] = format_git_commit_date_from_local_git(
  File "/home/runner/work/grass/grass/dist.x86_64-pc-linux-gnu/utils/mkhtml.py", line 345, in format_git_commit_date_from_local_git
    return datetime.fromisoformat(
ValueError: Invalid isoformat string: '2024-05-17T16:09:45Z'

https://github.com/OSGeo/grass/actions/runs/9131120968/job/25109628154?pr=3704#step:8:6469

Originally posted by @kritibirda26 in #3704 (comment)

@wenzeslaus
Copy link
Member Author

Older Python does not support Z while recent one does (3.8 does not, 3.11 does).

To test locally:

>>> datetime.fromisoformat('2011-11-04T00:05:23Z')

Given that it seems this changed on us, the change might be coming from configuration of the CI machine either for time zone or for version of Git. In the code, we are using something like:

git log --format="%aI"

This gives committer time with different time zones (not influenced by TZ variable). I get some +00:00 but not Z (Z is short for +00:00). %Ia is "author date, strict ISO 8601 format" according to the Git doc, but that can be Z or +00:00 as far as I understand.

$ git --version 
git version 2.25.1
$ python --version
Python 3.8.10

@wenzeslaus wenzeslaus added bug Something isn't working CI Continuous integration labels May 18, 2024
@wenzeslaus wenzeslaus added this to the 8.4.0 milestone May 18, 2024
@wenzeslaus
Copy link
Member Author

The latest version of Git is 2.45.1 (source) and it contains git/git@1f49f75:

The output format for dates "iso-strict" has been tweaked to show
a time in the Zulu timezone with "Z" suffix, instead of "+00:00".

* bb/iso-strict-utc:
  date: make "iso-strict" conforming for the UTC timezone

How it actually is with Z vs +00:00 is not clear to me (whether Z is actually more strict when +00:00 refers to the local time zone rather than UTC time...), but that's not important here.

So anyway, the CI actually has latest Git which produces Z and Python is 3.10 which does not support Z in fromisoformat (3.10).

We need to either 1) upgrade Python in CI, 2) downgrade Git in CI, or 3) deal in code with non-matching "freshness" of Python and Git.

Number 1 seems best but 3 seems okay too and is doable (try endswith and replace on ValueError).

@wenzeslaus
Copy link
Member Author

Note: Some PRs happily run (#3719). Maybe different Git on different VMs in CI...

@echoix
Copy link
Member

echoix commented May 18, 2024

https://github.com/actions/runner-images/releases

https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20240516.4

There is an image 20240516.4.1 (the one used in the linked run), that I see has git 2.45.1 instead of 2.43.2 (for the 20240510.1.0).

image

Images are changed almost weekly and can take up to a week to fully deploy, so, it's totally possible to have some running on different VMs, as it was yesterday.

You may want to see if there are other suspects in it.

At the same time, we can now use the ubuntu-24.04 tag, as beta, if we wanted to. I saw that they included a bit less of software for maintenance reasons, and less cached images. It at least has easy access to the newest gcc compiler.

@echoix
Copy link
Member

echoix commented May 18, 2024

You might also want to take a look at actions/runner-images#9883

@wenzeslaus
Copy link
Member Author

You might also want to take a look at actions/runner-images#9883

Thanks. This helped me to decide that we just need to accommodate the different versions.

@echoix echoix linked a pull request May 19, 2024 that will close this issue
@echoix echoix closed this as completed May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI Continuous integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants