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

Feature requestion: add "retry" option #321

Open
nmattia opened this issue Nov 5, 2021 · 7 comments
Open

Feature requestion: add "retry" option #321

nmattia opened this issue Nov 5, 2021 · 7 comments
Labels
status: pinned Should not be labeled as stale type: feature New feature or feature request

Comments

@nmattia
Copy link

nmattia commented Nov 5, 2021

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 nmattia added the status: pending More info is needed before deciding what to do label Nov 5, 2021
@EndBug
Copy link
Owner

EndBug commented Nov 5, 2021

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.

@nmattia
Copy link
Author

nmattia commented Nov 5, 2021

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/add-and-commit@v7.4.0
        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 nmattia closed this as completed Nov 5, 2021
@Twixes
Copy link

Twixes commented Feb 17, 2023

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?

@nmattia
Copy link
Author

nmattia commented Feb 23, 2023

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!

@Twixes
Copy link

Twixes commented Feb 23, 2023

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

@GreasyAvocado
Copy link

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?

@EndBug
Copy link
Owner

EndBug commented Oct 4, 2023

@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 EndBug reopened this Oct 4, 2023
@EndBug EndBug added type: feature New feature or feature request status: pinned Should not be labeled as stale and removed status: pending More info is needed before deciding what to do labels Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: pinned Should not be labeled as stale type: feature New feature or feature request
Projects
None yet
Development

No branches or pull requests

4 participants