+1 to the general proposal. I'm not bothered if something says a gradle command and in execution, that task ends up running multiple different commands. Arguably, if we're running a gradle task manualy to prepare for a subsequent task that latter task should be adding the former to it's dependencies.
Also agree that this is a post Jenkins exit task. One migration in an area at a time please. On Tue, Oct 10, 2023, 8:07 AM Kenneth Knowles <k...@apache.org> wrote: > 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 >>>>> >>>>
