Ken, I'm probably not seeing something but how does using the PreCommit as
a proxy improve upon just running the post commit via the phrase it already
supports ('Run Java PostCommit')?On Fri, Mar 9, 2018 at 11:22 AM, Kenneth Knowles <k...@google.com> wrote: > Indeed, we've already had the discussion a couple of times and I think the > criteria are clearly met. Incremental progress is a good thing and we > shouldn't block it. > > OTOH I see where Romain is coming from and I have a good example that > supports a slightly different action. Consider https://github.com/apache/ > beam/pull/4740 which fixes some errors in how we use dependency > mechanisms. > > This PR is green except that I need to fix some Maven pom slightly more. > That is throwaway work. I would love to just not have to do it. But > removing the precommit does not actually make the PR OK to merge. It would > cause postcommits to fail. > > We can hope such situations are rare. I think I tend to be hit by this > more often than most, as I work with the project build health quite a bit. > > Here is a proposal to support these things: instead of deleting the job in > #4814, move it to not run automatically but only via a phrase. In fact, you > could migrate it to be the manually-invoked version of the postcommit job > as we've discussed a couple times. Then if someone is working on the build > in something like #4740 they can invoke it manually. > > Kenn > > On Fri, Mar 9, 2018 at 10:25 AM Lukasz Cwik <lc...@google.com> wrote: > >> Based upon the criteria that was discussed on the mailing list[1], I >> would agree with Kenn about merging PR/4814 (drop Java Maven precommit). >> >> 1: https://lists.apache.org/thread.html/7eba5c77bc1a77b5046d915ab59f5f >> 6fc41536c2c84863ad2efb5e99@%3Cdev.beam.apache.org%3E >> >> On Thu, Mar 8, 2018 at 9:10 PM, Romain Manni-Bucau <rmannibu...@gmail.com >> > wrote: >> >>> Hi Kenneth, >>> >>> For now maven covers the full needs of beam. If we start to have this >>> kind of PR we become dependent of the 2 builds which is what this thread is >>> about avoiding so tempted to say it must be a PR drop completely maven or >>> nothing as mentionned before. >>> >>> Le 9 mars 2018 04:48, "Kenneth Knowles" <k...@google.com> a écrit : >>> >>>> I would like to briefly re-focus this discussion and suggest that we >>>> merge https://github.com/apache/beam/pull/4814. >>>> >>>> The only material objection I've heard is that it means the precommit >>>> no longer tests exactly what is built for release. It is a valid concern, >>>> but we have mvn postcommit so the coverage is not lost. The benefits of >>>> quicker reviews and less Jenkins congestion seem worth it to me. >>>> >>>> Kenn >>>> >>>> On Thu, Mar 8, 2018 at 12:33 PM Romain Manni-Bucau < >>>> rmannibu...@gmail.com> wrote: >>>> >>>>> @Luskasz: not sure Im the best to host it since I know more gradle >>>>> internals that user interface/ecosystem but happy to help. Will also >>>>> require a "sudo" merger for this day to merge fixes asap - guess we can >>>>> bypass reviews or have a fast cycle plan for this day to avoid it to be a >>>>> week? >>>>> >>>>> Le 8 mars 2018 21:08, "Kenneth Knowles" <k...@google.com> a écrit : >>>>> >>>>> @Romain - Idea/IntelliJ is great with Gradle, way better than Maven, >>>>> and what I mean is that I have added enough hints that it works OOTB >>>>> already. The rest of my instructions are just how you should override >>>>> IntelliJ's defaults to have a proper dev env - mostly just about storing >>>>> files outside the git clone. >>>>> >>>>> >>>>> Hmm it is always super slow here and not as integrated as maven which >>>>> is smooth thanks to the combination of idea build and maven plugin config >>>>> read. Import is also faster cause it just reads meta and doesnt run >>>>> anything. Hope it is a "a few times" issues at the moment but not yet >>>>> sure. >>>>> >>>>> >>>>> >>>>> @Łukasz - yea I just mean the ones that I find in my email and >>>>> browsing https://builds.apache.org/view/A-D/view/Beam/ which are the >>>>> ones you classify as "totally not working". I would love to disable these. >>>>> >>>>> On the subject of running things on a pending PR - we should not run >>>>> postcommit jobs on PRs. We should make separate (optional) precommit jobs >>>>> that run the same build commands. That will give a more clear history and >>>>> allow trivial configuration of which jobs deserve alert emails and which >>>>> are not a problem. This is easy but I've been waiting to do it after >>>>> Gradle >>>>> migration. >>>>> >>>>> On Thu, Mar 8, 2018 at 11:37 AM Lukasz Cwik <lc...@google.com> wrote: >>>>> >>>>>> Romain, would you like to host/plan/run the Gradle fixit day? >>>>>> >>>>>> On Thu, Mar 8, 2018 at 11:24 AM, Chamikara Jayalath < >>>>>> chamik...@google.com> wrote: >>>>>> >>>>>>> +1 for the general idea of fixit day/week for Gradle. >>>>>>> >>>>>>> Agree with what Łukasz said. Some of these performance tests are new >>>>>>> and are flaky due to other issues that were discovered during the >>>>>>> process >>>>>>> of adding the test. >>>>>>> >>>>>>> I think the high level blocker is updating performance testing >>>>>>> framework to use Gradle. This will involve adding Gradle-based logic for >>>>>>> invoking PerfKitBenchmaker, for example [1], and updating >>>>>>> PerfKitBenchmarker to issue a Gradle command for running the benchmark >>>>>>> [2]. >>>>>>> First task will be to find the work needed here and updating the >>>>>>> relevant >>>>>>> JIRA [3]. >>>>>>> >>>>>>> Thanks, >>>>>>> Cham >>>>>>> >>>>>>> [1] https://github.com/apache/beam/blob/master/sdks/ >>>>>>> java/io/google-cloud-platform/pom.xml#L79 >>>>>>> [2] https://github.com/GoogleCloudPlatform/PerfKitBenchmarker/blob/ >>>>>>> master/perfkitbenchmarker/beam_benchmark_helper.py >>>>>>> [3] https://issues.apache.org/jira/browse/BEAM-3251 >>>>>>> >>>>>>> On Thu, Mar 8, 2018 at 3:10 AM Łukasz Gajowy < >>>>>>> lukasz.gaj...@gmail.com> wrote: >>>>>>> >>>>>>>> 2018-03-07 22:29 GMT+01:00 Kenneth Knowles <k...@google.com>: >>>>>>>> >>>>>>>>> >>>>>>>>> Based on https://builds.apache.org/view/A-D/view/Beam/ and our >>>>>>>>> failure spam level the performance tests are mostly not healthy >>>>>>>>> anyhow. So >>>>>>>>> is there any high level blocker to switching them or is it just >>>>>>>>> someone >>>>>>>>> sitting down with each one? >>>>>>>>> >>>>>>>>> >>>>>>>> I thought I could share my point of view here as I am working on >>>>>>>> the Performance Test part for a while now. I wouldn't say those are >>>>>>>> "mostly >>>>>>>> not healthy". The situation is as follows: >>>>>>>> >>>>>>>> - totally not working: Python, Spark performance tests (don't know >>>>>>>> why, I'm not maintaining those tests. Should we disable them?) >>>>>>>> - flaky: the recently re-enabled JDBC Performance Test. It's seems >>>>>>>> to be flaky mostly due to: https://issues.apache.org/ >>>>>>>> jira/browse/BEAM-3798 >>>>>>>> - working well, rarely failing: AvroIO, TextIO, Compressed Text, >>>>>>>> TFRecordIO >>>>>>>> >>>>>>>> Also, some test failures are caused due to pending PRs (we >>>>>>>> sometimes run concrete tests to check if PRs won't break them). This >>>>>>>> also >>>>>>>> causes failures sometimes. >>>>>>>> >>>>>>>> I can help with switching the Performance tests to Gradle as this >>>>>>>> part seems to be free to take. >>>>>>>> >>>>>>>> >>>>>>>> Łukasz >>>>>>>> >>>>>>>> >>>>>> >>>>> >>
