add-and-commit icon indicating copy to clipboard operation
add-and-commit copied to clipboard

Feature requestion: add "retry" option

Open nmattia opened this issue 4 years ago • 8 comments

It would be great to be able to retry. My use case is that I have different workflows that push to a PR with pull: --rebase. However it can happen that Workflow 1 pulls, then workflow 2 pulls, then workflow 1 pushes, and then workflow 2 will fail; even though it did rebase, it rebased too early.

nmattia avatar Nov 05 '21 13:11 nmattia

I don't know how I feel about this: I get your situation, but wouldn't it be better to execute those jobs one after the other in the first place? There's a needs options you can use to make one job start only after one has ended: this way you'll be sure to run the second one with the correct repo.

EndBug avatar Nov 05 '21 13:11 EndBug

Thanks for the quick reply,

Both steps that uses add-and-commit actually are the "same" step, they are just executed as part of a different matrix combination. Here's an example:

name: screenshots

jobs:
  screenshots:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        device: [ 'desktop', 'mobile' ]

    steps:
      - uses: actions/checkout@v2

      - name: generate screenshots
        run: ./generate-screenshots ${{ matrix.device }}

      - name: Commit screenshots
        uses: EndBug/[email protected]
        with:
          message: "Update screenshots for ${{ matrix.device }}"
          # --rebase: make sure both the mobile and desktop screenshots can be
          # uploaded
          pull: "--rebase"

I like the needs solution, although AFAICT I can only use that for a job. Also I do want to run the generation part concurrently, I only want to commit sequentially. I think I'll try to extract the add-and-commit into a new job that aggregates the results from the other jobs. Thanks!

nmattia avatar Nov 05 '21 15:11 nmattia

I'm running into the exact situation (committing UI screenshots updated across a few browsers in a matrix)! How did you sort it out @nmattia?

Twixes avatar Feb 17 '23 15:02 Twixes

Oh it's been a while! If I (re) understand the issue and solution correctly, I ended up doing this:

I think I'll try to extract the add-and-commit into a new job that aggregates the results from the other jobs.

In particular, here's the matrix job (in your case you'd probably replace desktop/mobile for firefox/chrome/whatever)

  # Job that starts the showcase and takes a screenshot of every page
  screenshots:
    runs-on: ubuntu-latest
    strategy:
      matrix:
        device: [ 'desktop', 'mobile' ]
      # Make sure that one failing test does not cancel all other matrix jobs
      fail-fast: false

and here's the "aggregator":

      # Download the desktop screenshots artifacts
      - uses: actions/download-artifact@v3
        with:
          name: e2e-screenshots-desktop
          path: screenshots/desktop

      # Download the mobile screenshots artifacts
      - uses: actions/download-artifact@v3
        with:
          name: e2e-screenshots-mobile
          path: screenshots/mobile

      - name: Commit screenshots
        uses: EndBug/add-and-commit@v9
        with:
          add: screenshots
          default_author: github_actions
          message: "🤖 Selenium screenshots auto-update"

Here's the workflow for reference: dfinity/internet-identity/.github/workflows/frontend-check.yml

Let me know if you have any questions!

nmattia avatar Feb 23 '23 16:02 nmattia

Ah, communicating via artifacts, that's clever! Thanks

Twixes avatar Feb 23 '23 16:02 Twixes

Hi @EndBug Any chance to reconsider this?

I have a big mono-repo, which many different workflows push to. Unfortunately I don't have a way to "sync" the workflows, or share artifacts between them, as suggested above.

Adding a retry_push: <bool> flag, and maybe also retry_push_attempts: <int>, would be amazing 🙏

For example:

- name: retry on push error
  uses: EndBug/add-and-commit@v9
  with:
    github_token: ${{ secrets.TOKEN }}
    message: "message"
    pull: --autostash
    push: true
    retry_push: true
    retry_push_attempts: 3

WDYT?

GreasyAvocado avatar Oct 01 '23 09:10 GreasyAvocado

@GreasyAvocado Since it seems like a lot of people would find it useful, I guess it's worth implementing it 😄

I'll reopen this issue and put it as pinned so it doesn't go stale, but I can't promise I'll have the time to make it happen soon. However, if somebody wants to open a PR, I'd suggest adding only something like push_attempts and make it 1 by default ;)

EndBug avatar Oct 04 '23 13:10 EndBug

any update on this one?

Amit-Baranes-Deel avatar Jul 22 '24 12:07 Amit-Baranes-Deel