search

upsidr / merge-gatekeeper

Get better merge control
MIT License
85 stars 14 forks source link

Increase limit job #42

Closed yuriydzobak closed 1 year ago

yuriydzobak commented 1 year ago

Hi, I faced an issue When PR has more than 30 jobs the merge-gatekeeper replace some job in the array list and if some job fails the MG can skip because that job is not in the list

  Total job count:     30
    jobs: ["unit-tests (test1)" "unit-tests (test2)" "unit-tests (test3)" "unit-tests (test4)" "unit-tests (test5)" "unit-tests (test6)" "unit-tests (test7)" "unit-tests (test8)" "unit-tests (test9)" "unit-tests (test10)" "unit-tests (test11)" "unit-tests (test12)" "unit-tests (test13)" "unit-tests (test14)" "unit-tests (test15)" "unit-tests (test16)" "unit-tests (test17)" "unit-tests (test18)" "unit-tests (test19)" "unit-tests (test20)" "unit-tests (test21)" "unit-tests (test22)" "unit-tests (test23)" "unit-tests (test23)" "unit-tests (test24)" "unit-tests (test25)" "unit-tests (test25)" "unit-tests (test26)" "unit-tests (test27)" "unit-tests (test28)"]
  Completed job count: 29
    jobs: ["unit-tests (test1)" "unit-tests (test2)" "unit-tests (test4)" "unit-tests (test5)" "unit-tests (test6)" "unit-tests (test7)" "unit-tests (test8)" "unit-tests (test9)" "unit-tests (test10)" "unit-tests (test11)" "unit-tests (test12)" "unit-tests (test13)" "unit-tests (test14)" "unit-tests (test15)" "unit-tests (test16)" "unit-tests (test17)" "unit-tests (test18)" "unit-tests (test19)" "unit-tests (test20)" "unit-tests (test21)" "unit-tests (test22)" "unit-tests (test23)" "unit-tests (test23)" "unit-tests (test24)" "unit-tests (test25)" "unit-tests (test25)" "unit-tests (test26)" "unit-tests (test27)" "unit-tests (test28)"]
  Failed job count:    0
    jobs: []

Finish validator: merge-gatekeeper processing.

  WARNING: Validation is yet to be completed. This is most likely due to some other jobs still running.
           Waiting for 15 seconds before retrying.

Start processing validator: merge-gatekeeper....
30 out of 30

  Total job count:     30
    jobs: ["Services Unit Tests" "unit-tests (test1)" "unit-tests (test2)" "unit-tests (test3)" "unit-tests (test4)" "unit-tests (test5)" "unit-tests (test6)" "unit-tests (test7)" "unit-tests (test8)" "unit-tests (test9)" "unit-tests (test10)" "unit-tests (test11)" "unit-tests (test12)" "unit-tests (test13)" "unit-tests (test14)" "unit-tests (test15)" "unit-tests (test16)" "unit-tests (test17)" "unit-tests (test18)" "unit-tests (test19)" "unit-tests (test20)" "unit-tests (test21)" "unit-tests (test22)" "unit-tests (test23)" "unit-tests (test23)" "unit-tests (test24)" "unit-tests (test25)" "unit-tests (test25)" "unit-tests (test26)" "unit-tests (test27)"]
  Completed job count: 30
    jobs: ["Services Unit Tests" "unit-tests (test1)" "unit-tests (test2)" "unit-tests (test3)" "unit-tests (test4)" "unit-tests (test5)" "unit-tests (test6)" "unit-tests (test7)" "unit-tests (test8)" "unit-tests (test9)" "unit-tests (test10)" "unit-tests (test11)" "unit-tests (test12)" "unit-tests (test13)" "unit-tests (test14)" "unit-tests (test15)" "unit-tests (test16)" "unit-tests (test17)" "unit-tests (test18)" "unit-tests (test19)" "unit-tests (test20)" "unit-tests (test21)" "unit-tests (test22)" "unit-tests (test23)" "unit-tests (test23)" "unit-tests (test24)" "unit-tests (test25)" "unit-tests (test25)" "unit-tests (test26)" "unit-tests (test27)"]
  Failed job count:    0
    jobs: []

As you see logs, the unit-tests (test28) was replaced Services Unit Tests.

It would be nice if the MK has an additional flag to increase the limit of jobs, like job-limit: 50

yuriydzobak commented 1 year ago

Hi @rytswd could you help with it?

rytswd commented 1 year ago

Hi @yuriydzobak, thanks for raising the issue! ☺️ We will certainly need to look at this more closely, as this is new to me at least. Let us dive deep and figure out what's possibly causing this - depending on that, we may be able to ensure Merge Gatekeeper works with any number of jobs, or we may introduce another field, with which you can configure based on your requirements 👍

yuriydzobak commented 1 year ago

Hi @yuriydzobak, thanks for raising the issue! ☺️ We will certainly need to look at this more closely, as this is new to me at least. Let us dive deep and figure out what's possibly causing this - depending on that, we may be able to ensure Merge Gatekeeper works with any number of jobs, or we may introduce another field, with which you can configure based on your requirements 👍

Would be nice if MG works with any number, because I have repos with 20-30 jobs, some repo has 70 jobs(matrix) and for example, users don't care about any limits in MG for me, any solution would be great 😂

Thank you

rytswd commented 1 year ago

@yuriydzobak Sorry for getting back to you late! Understood, we will do further testing on our side. Merge Gatekeeper aims to be a drop-in solution without any extra configurations for most standard monorepo use cases, so we would like it to handle cases where there are so many jobs running at the same time. We will check and get back to you as soon as we can!

rytswd commented 1 year ago

Sorry for the delay in getting back to this, just a quick update on this. We have found that the GitHub API we use indeed only gives us 30 results, and this can be increased to 100. However, this is more of a pagination handling by GitHub, and in order to fully support any number of jobs, we would need to adjust our API usages in a bit more complicated manner (i.e. ensure pagination is taken into account). We think we have an implementation idea to fix this, but please bear with us until we work out the solution with proper tests in place 🙏

rytswd commented 1 year ago

Given the above fix #48 is in, this should now be handled without any extra configurations. Merge Gatekeeper will simply follow the pagination to ensure we get the full list of jobs. Thanks to @Jrc356 for contributing a fix! 👍