Only do this workflow if it is on the CodeForPhilly owner of this rep…

[dherbst]

Forks by individuals should not try to deploy to staging or production.

[themightychris]

ideally, folks working on forks should be able to work on actions too and the forks just fail to deploy without secrets in place. If they're doing work in topic branches on their fork to merge upstream they wouldn't burn actions minutes

[dherbst]

@themightychris I agree with you. I wanted to stop the deploy action from running on forks. That's what this does if it wasn't clear.

[themightychris]

yeah but this approach defeats one of the benefits of github actions-driven deployments that forks can have a full CI process just by popping in their own secrets.

Contributors could instead just be instructed to work from topic branches in their forks instead of right on main/staging, which is a best practice anyway. Granted though, it's hard to get that training out to everyone

[dherbst]

If you are going to deploy to production or staging from your fork, I'd say that will probably create some problems especially with communication to the rest of the team members. Nothing stops you from running other actions, just this one. The way it was before adding the if statement to check the repo owner, it was running anyway, using up seconds of actions quota, and then notifying the fork owner that the deployment failed. That notification is somewhat confusing, and can be avoided by only doing this deployment action on the CodeForPhilly repo.

If for some reason, you want to test the deployment action, just comment out the if statement. If you are on a topic branch, you'll have to modify the action anyway because of the way it was written to only execute on master and staging.

[dherbst]

Because we can't use secrets.NAME in the if: to gate the step, we can do this in the shell:

          if [ -n "${{ secrets.DEFAULT_USER }}" ]; then
                proceed as normal
          fi

so if you have some alternate prod/staging set up with secrets, then we can do that.

[machow]

@dherbst do you mind if we undo this guard in the action? I think public github repos have "unlimited" action minutes, so running even a failing action hopefully shouldn't cause issues. Maybe we can add documentation to the README on deployment? (I basically have failed to document anything about this, so can see how the action running and failing could be confusing to people 😅).

There's no rush here, since we're still fleshing out the pipe--happy to wait and discuss more in a Tues project meeting.

[machow]

Ah, from talking more with @dherbst--it sounds like the big problem this solves is that github will spam failure emails to people who fork. It's probably much more important to tune to the people forking right now (@dherbst and @chriscardillo ?!), but I've opened an issue for researching how to hit both the contributor and forker use-cases.

https://github.com/CodeForPhilly/cfp-data-pipeline/issues/36

[dherbst]

I'll change it instead to gate on the existence of the secrets - because having it notify a failure isn't optimal. As I pointed out above, the way to do this is to put this in the run steps - unfortunately we cannot test existence of a secret at this time in the if: statement.

          if [ -n "${{ secrets.DEFAULT_USER }}" ]; then
                proceed as normal
          fi

[machow]

Either way--no rush. I think it might be possible to set a variable in $GITHUB_ENV (see here), and then read it out in the context using ${{env.<variable_name>}} (see here), but I'm not totally sure and realize it's a pain to fiddle with 😬.

If that's the case though, could run a task that sets echo "HAS_SECRETS=<some_value>" >> $GITHUB_ENV or something, and then uses if: ${{env.HAS_SECRETS}}?!

Totally on board with punting this down the road :p

[dherbst]

I have the revised PR - I'll put it up in a minute.

[dherbst]

https://github.com/CodeForPhilly/cfp-data-pipeline/pull/37 this is the revised PR.