Hide currently known failures from the summary dashboards #619
Conversation
workspace/workflows/jobs.py
Outdated
| wf_conclusions = { | ||
| k: v | ||
| for k, v in wf_conclusions.items() | ||
| if v == "success" or int(k) not in known_failure_ids |
There was a problem hiding this comment.
Seeing values being converted between types in the middle of a function like this is a bit of a code smell to me. In this case, it suggests we're not clear whether the workflow IDs should be ints or (presumably) strs, which means there's a risk we'll get them mixed up. Ideally, we'd do any necessary conversion at the boundary of a system -- here this would either involve converting str->int in RepoWorkflowReporter, or converting str->int when loading WORKFLOWS_KNOWN_TO_FAIL, or insisting that WORKFLOWS_KNOWN_TO_FAIL contains strs.
No need to fix now if it's not straightforward, but something to bear in mind in future.
There was a problem hiding this comment.
This int() is not actually needed - I've removed it. Workflow IDs are int except when read/writing to/from the cache (due to json.dumps) - this is me forgetting that. I'll do some tidying in a later PR so that type conversions are only at the boundary of the cache.
alarthast
force-pushed
the
benbc/ignore-some-failures
branch
from
4746899
to
4b5726b
Compare
These are expected to break from time to time and we have other mechanisms for letting us know about it. (At the time of writing Steve checks these himself, but soon this will be automated and handed over to Tech Support.)
alarthast
force-pushed
the
benbc/ignore-some-failures
branch
from
4b5726b
to
c7da56e
Compare
alarthast
force-pushed
the
benbc/ignore-some-failures
branch
from
c98c928
to
75331b5
Compare
|
Post-approval changes
|
|
Apologies that I forgot to squash this 😭 , I'll remember next time |
config.pyand hide them when creating summariestargetis a specific repo, not a summaryIt would be better to make this an optional configuration - explore in #618
Previous iteration of this PR: #617