Upgrade fedora to version 40 in GHA CI #1502
Conversation
.github/workflows/build.yaml
Outdated
| @@ -583,7 +583,7 @@ jobs: | |||
| # # unittest coverage | |||
| # - os: ubuntu-22.04 | |||
| # container: 'fedora' | |||
| # containerTag: 39 | |||
| # containerTag: 30 | |||
There was a problem hiding this comment.
Although this has line has been commented out, containerTag must be 40 ?
There was a problem hiding this comment.
woopsie - good catch. Fix upcoming.
| @@ -535,6 +535,8 @@ def on_message(self, event): | |||
| router = 'C' | |||
| elif event.receiver == self.routers['D']['mgmt_receiver']: | |||
| router = 'D' | |||
| else: | |||
| raise Exception("Unexpected event receiver") | |||
There was a problem hiding this comment.
which linter warning made you remove these raise Exception?
There was a problem hiding this comment.
"router" may be used before initialization.
Which isn't true for the code as it stands now, but I'd rather not suppress the error in case we modify the test at some point and miss updating the if statement.
There was a problem hiding this comment.
router may be used before initialization
From the actual context this doesn't appear possible, but yeah there is a hole in the code where router is never set. Rather than suppress the warning I added the Exception as a "defensive coding" approach. This test has a history of being somewhat flaky so I'm sure we'll be hacking at it at some point in the future. Should we change the code in some way where we add a new event receiver then the existing if statement would fail silently and router would in fact be uninitialized. The exception would prevent that foot-gun.
| hacking>=6.1.0 | ||
| flake8>=6.1.0 | ||
| pylint>=3.0.3 | ||
| mypy>=1.8.0 |
There was a problem hiding this comment.
I guess you realize what this means (different people with different versions of these tools will get different linter warnings), so no need for me to point this out.
Besides, as things now stand, it is already true that sometimes the os versions and sometimes the pypi venv versions of these tools are used, depending on how one sets up their dev environment, so this is not actually something entirely new. What's now new is that this will be also seen in CI (new ci failures on python liner checks because pypi packages were updated, without any code changes in router)
There was a problem hiding this comment.
Yeah - this change is a bit heavy handed. I had to advance pylint (I think it was pylint) as the 3.0.3 version was actually failing on F40 (some type assertion).
Having newer linter versions catch new errors is a good thing... probably. If it introduces lots of noise then yeah we should set upper version bounds as necessary.
No description provided.