Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase limit job #42

Closed
yuriydzobak opened this issue Oct 12, 2022 · 6 comments
Closed

Increase limit job #42

yuriydzobak opened this issue Oct 12, 2022 · 6 comments

Comments

@yuriydzobak
Copy link

yuriydzobak commented Oct 12, 2022

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
Copy link
Author

Hi @rytswd
could you help with it?

@rytswd
Copy link
Contributor

rytswd commented Oct 19, 2022

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
Copy link
Author

yuriydzobak commented Oct 19, 2022

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
Copy link
Contributor

rytswd commented Oct 24, 2022

@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
Copy link
Contributor

rytswd commented Oct 29, 2022

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
Copy link
Contributor

rytswd commented Nov 26, 2022

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! 👍

@rytswd rytswd closed this as completed Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants