damccorm commented on code in PR #29270: URL: https://github.com/apache/beam/pull/29270#discussion_r1380354617
########## .test-infra/metrics/sync/github/sync_workflows.py: ########## @@ -47,6 +47,330 @@ GH_NUMBER_OF_WORKFLOW_RUNS_TO_FETCH =\ os.environ['GH_NUMBER_OF_WORKFLOW_RUNS_TO_FETCH'] +# Maps workflows to dashboard category. Any workflows not in one of these lists +# will get auto-mapped to misc. +CORE_JAVA_TESTS = [ + 'PreCommit SQL Java17', Review Comment: I disagree; basically, my conclusions after seeing the results of my first pass were: 1) It is pretty much impossible to create a rules based mapping that gets us what we want. A good example is the CORE_INFRA category. I think that having a single panel for viewing our important infra is really useful or rotations like plat-mon or for individuals who are interested (including me 😃 ). Another good example is IO Performance tests which probably belong with Java, but not necessarily. 2) I don't want to be the only person maintaining this mapping (and I don't want you to be the only other person). The easiest way to make that true is to make it trivial to edit. This does that. This also maps to how we've maintained dashboards internally, which has worked reasonably well. > If it doesn't then I think what need to do is to standardize test naming This has 2 downsides: 1) It is much harder to change your categories in the future if they are tied to test names. 2) I think it is much less understandable to someone who is not familiar with the automation. Admittedly, we can doc it. --------------------------------------------------------- Long term, it would be nice to move this to a config and have a workflow that verifies that test names are all represented in the config. That would actually be my preferred solution and I think this moves things in that direction. If we standardized on naming, we would need to build in a similar mechanism to enforce naming; so I don't think this is much worse. Without automation, it doesn't really matter how we do it, we're going to get out of sync. ########## .test-infra/metrics/sync/github/sync_workflows.py: ########## @@ -47,6 +47,330 @@ GH_NUMBER_OF_WORKFLOW_RUNS_TO_FETCH =\ os.environ['GH_NUMBER_OF_WORKFLOW_RUNS_TO_FETCH'] +# Maps workflows to dashboard category. Any workflows not in one of these lists +# will get auto-mapped to misc. +CORE_JAVA_TESTS = [ + 'PreCommit SQL Java17', Review Comment: I disagree; basically, my conclusions after seeing the results of my first pass were: 1) It is pretty much impossible to create a rules based mapping that gets us what we want. A good example is the CORE_INFRA category. I think that having a single panel for viewing our important infra is really useful or rotations like plat-mon or for individuals who are interested (including me 😃 ). Another good example is IO Performance tests which probably belong with Java, but not necessarily. 2) I don't want to be the only person maintaining this mapping (and I don't want you to be the only other person). The easiest way to make that true is to make it trivial to edit. This does that. This also maps to how we've maintained dashboards internally, which has worked reasonably well. > If it doesn't then I think what need to do is to standardize test naming This has 2 downsides: 1) It is much harder to change your categories in the future if they are tied to test names. 2) I think it is much less understandable to someone who is not familiar with the automation. Admittedly, we can doc it. --------------------------------------------------------- Long term, it would be nice to move this to a config and have a workflow that verifies that test names are all represented in the config. That would actually be my preferred solution and I think this moves things in that direction. If we standardized on naming, we would need to build in a similar mechanism to enforce naming; so I don't think this is much worse. Without automation, it doesn't really matter how we do it, we're going to get out of sync. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
