On Tue, Oct 10, 2023 at 10:21 AM Danny McCormick via dev < dev@beam.apache.org> wrote:
> I'm +1 on: > - standardizing our naming > - making job names match their commands exactly (though I'd still like the > `Run` prefix so that you can do things like say "Suite XYZ failed" without > triggering the automation) > - removing pre/postcommit from the naming (we actually already run many > precommits as postcommits as well) Fully agree with your point of keeping "Run" as the magic word and the way we have it today. I'm -0 on: > - Doing this immediately - I'd prefer we wait til the Jenkins to Actions > migration is done and we can do this in bulk versus renaming things as we > go since we're so close to the finish line and exact parity makes reviews > easier. Cool. And indeed this is why I didn't "just do it' (aside from this being enough impact to people's daily lives that I wanted to get feedback from dev@). If we can fold it in as a last step to the migration, that would be a nice-to-have. Otherwise ping back when ready please :-) On Tue, Oct 10, 2023 at 11:15 AM Yi Hu via dev <dev@beam.apache.org> wrote: > Thanks for raising this. This generally works, though some jobs run more > than one gradle task (e.g. some IO_Direct_PreCommit run both :build (which > executes unit tests) and :integrationTest). > On Tue, Oct 10, 2023 at 10:21 AM Danny McCormick via dev < dev@beam.apache.org> wrote: > I'm -1 on: > - Tying the naming to gradle - like Yi mentioned some workflows have > multiple gradle tasks, some don't have any, and I think that's ok. > Just to clarify: I'm not proposing tying them to gradle tasks (I'm fine with `go test` for example) or doing this in situations where it is unnatural. My example probably confused this because I left off the `./gradlew` just to save space. I'm proposing naming them after their obvious repro command, wherever applicable. Mostly fixing stuff like how the status label "Google Cloud Dataflow Runner V2 Java ValidatesRunner Tests (streaming)" is literally a less useful way of writing ":runners:google-cloud-dataflow-java:validatesRunnerV2Streaming". FWIW I think Yi's example demonstrates an anti-pattern (mixing hermetic/reliable and non-hermetic/possibly-flaky tests in one test signal) but I'm sure there are indeed jobs where this doesn't make sense. Kenn > > Thanks, > Danny > > >> >> Another option is to normalize the naming of every job, saying the job >> name is X, then workflow name is PreCommit_X or PostCommit_X, and the >> phrase is Run X. Currently most PreCommit follow this pattern, but there >> are also many outliers. A good start could be to clean up all jobs >> to follow the same pattern. >> >> >> On Tue, Oct 10, 2023 at 9:57 AM Kenneth Knowles <k...@apache.org> wrote: >> >>> FWIW I aware of the README in >>> https://github.com/apache/beam/tree/master/.test-infra/jenkins that >>> lists the phrases alongside the jobs. This is just wasted work to maintain >>> IMO. >>> >>> Kenn >>> >>> On Tue, Oct 10, 2023 at 9:46 AM Kenneth Knowles <k...@apache.org> wrote: >>> >>>> *Proposal:* make all the job names exactly match the GH comment to run >>>> them and make it also as close as possible to how to reproduce locally >>>> >>>> *Example problems*: >>>> >>>> - We have really silly redundant jobs results like 'Chicago Taxi >>>> Example on Dataflow ("Run Chicago Taxi on Dataflow")' and >>>> 'Python_Xlang_IO_Dataflow ("Run Python_Xlang_IO_Dataflow PostCommit")' >>>> >>>> - We have jobs that there's no way you could guess the command 'Google >>>> Cloud Dataflow Runner V2 Java ValidatesRunner Tests (streaming)' >>>> >>>> - (nit) We are weirdly inconsistent about using spaces vs underscores. >>>> I don't think any of our infrastructure cares about this. >>>> >>>> *Extra proposal*: make the job name also the local command, where >>>> possible >>>> >>>> *Example: * >>>> https://github.com/apache/beam/blob/master/.github/workflows/beam_PostCommit_Java_ValidatesRunner_Dataflow.yml >>>> >>>> - This runs :runners:google-cloud-dataflow-java:validatesRunner >>>> - So make the status label >>>> ":runners:google-cloud-dataflow-java:validatesRunner" >>>> - "Run :runners:google-cloud-dataflow-java:validatesRunner" as comment >>>> >>>> If I want to run it locally, yes there are GCP things I have to set up, >>>> but I know the gradle command now. >>>> >>>> *Corollary*: remove "postcommit" and "precommit" from names, because >>>> whether a suite runs before merge or after merge is not a property of the >>>> suite. >>>> >>>> *Caveats*: I haven't been that involved. I didn't do this to Jenkins >>>> because they are going away. I didn't do anything to GHA because I don't >>>> know if they are ready or in flux. >>>> >>>> I know this is the sort of thing that invites bikeshedding. It just >>>> would save me a few minutes when puzzling out what to care about and how to >>>> kick jobs on the release branch validation PR. >>>> >>>> I'm happy to scrape through the existing stuff and align it. Perfect >>>> task for when my brain is too tired for other work. >>>> >>>> Kenn >>>> >>>
