chore(ci): on demand review app #736
Conversation
|
|
|
/deploy defis |
|
Code for comment trigger needs to be on |
| required: true | ||
| default: 'ecospheres' | ||
| type: choice | ||
| options: |
| workflow_dispatch: | ||
| inputs: | ||
| site: | ||
| description: 'Site to deploy (ecospheres, meteo-france, or logistique)' |
There was a problem hiding this comment.
Description doesn't match options. I'd remove them to avoid syncing issues.
| - name: Set environment variables | ||
| run: | | ||
| if [ "${{ github.event_name }}" = "pull_request" ]; then | ||
| echo "SITE_ID=${{ github.event.pull_request.head.ref == 'main' && 'ecospheres' || 'ecospheres' }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
Doesn't that always result in 'ecospheres'?
| uses: dokku/github-action@master | ||
| # ignore errors as the app might already exist on workflow_dispatch | ||
| continue-on-error: true |
There was a problem hiding this comment.
Wouldn't it be safer to split the two cases (workflow dispatch vs pr deploy) then?
| jobs: | ||
| process_comment: | ||
| runs-on: ubuntu-latest | ||
| if: ${{ github.event.issue.pull_request && contains(github.event.comment.body, '/deploy') }} |
There was a problem hiding this comment.
| if: ${{ github.event.issue.pull_request && contains(github.event.comment.body, '/deploy') }} | |
| if: ${{ github.event.issue.pull_request && startsWith(github.event.comment.body, '/deploy') }} |
?
| } | ||
| }); | ||
| } catch (error) { | ||
| deploymentMessage += `❌ Failed to trigger deployment for ${site}: ${error.message}\n`; |
There was a problem hiding this comment.
| deploymentMessage += `❌ Failed to trigger deployment for ${site}: ${error.message}\n`; | |
| deploymentMessage += `❌ Failed to trigger deployment for '${site}': ${error.message}\n`; |
| } | ||
| } | ||
|
|
||
| if (!validSitesFound) { |
There was a problem hiding this comment.
Not sure that makes sense. If I /deploy a couple sites at once, I'd want to know about any invalid site, not just when all of them are.
There was a problem hiding this comment.
IIUC, we'd now only deploy "ecospheres" by default, and all other sites would require /deploy. I'm tempted to have a list of defaults sites to deploy instead of just "ecospheres", but maybe not now...
There was a problem hiding this comment.
Would be cool if new commits triggered redeploy for sites that were previously /deploy'ed. I don't see a simple, obvious way to do it, so probably later.
| AVAILABLE_SITES="${{ env.AVAILABLE_SITES }}" | ||
|
|
||
| # Extract sites from command | ||
| if [[ $COMMENT =~ /deploy\ +([a-zA-Z0-9,-]+) ]]; then |
There was a problem hiding this comment.
That comma got me wondering for a while. I'd move it up front and possibly add a comment? Or go with space-delimited site names?
Fix ecolabdata/ecospheres#566
Ne déploie par défaut que le site
ecospheres. Les autres sites peuvent être déployés via un commentaire sur la PR/deploy {site}.Cela permet d'alléger la charge sur le serveur qui héberge les preview apps et ne pas faire de déploiement inutile.
Evidemment, cela nécessite une action manuelle vs un déploiement auto de toutes les verticales. Il faut aussi redéployer après le dernier commit le cas échéant.
Disclaimer 🤖 : le workflow
review-app-comment-trigger.ymla été largement écrit par une IA.